Skip to content

interface: Expose load utxo snapshot functionality #869

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

Conversation

D33r-Gee
Copy link

@D33r-Gee D33r-Gee commented Apr 28, 2025

Expose load/activate AssumeUTXO snapshot functionality so that it can be loaded through the GUI.

This can be tested and viewed in action in the qml repository with legacy code (bitcoin-core/gui-qml#424)

Also for further examination please check out the POC branch

POC Ubuntu Screenshots `signet`

Screenshot 2025-04-28 100528
Launch bitcoin-qt on signet
Screenshot 2025-04-28 100541
Navigate to Settings -> Options
Screenshot 2025-04-28 100559
Click the "Load Snapshot..." button
Select_file
Navigate to where your snapshot file is. The snapshot was downloaded from here
Screenshot 2025-04-28 100735
Click "Yes"
Screenshot 2025-04-28 100950
Wait for the snapshot to load and for this pop-up to appear and click "Ok"
Screenshot 2025-04-28 101743
Verify the "chain_snapshot" directory is present in your datadir

Expose load/activate AssumeUTXO snapshot functionaility so that it can be laoded trhough the GUI.
@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 28, 2025

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

@D33r-Gee
Copy link
Author

friendly ping @Sjors

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.

There's no consumer of this on the bitcoin-core/gui side, can you show this works with some sort of POC on the bitcoin-core/gui side?

How do we know this works

@D33r-Gee
Copy link
Author

can you show this works with some sort of POC on the bitcoin-core/gui side?

Great idea! Just updated the description with a link to a POC and screenshots...

@Sjors
Copy link
Member

Sjors commented Apr 29, 2025

@D33r-Gee I'm a bit confused what the plan is here. Do you intend to implement this feature in the current GUI, or only in the QML project?

If the plan is to implement it here, which would be great, I would suggest opening two pull requests:

  1. The entire feature, including the interface change commit, here - but marked as draft
  2. The interface change commit, against the Bitcoin Core repo. Conceptually this in an interface ON the node FOR the GUI, so it belong in that repo. From there you would link to this PR, so reviewers understand how it's used.

Just having an interface change commit without the code that uses it, probably won't get merged. It might be fine to have a working demo in the QML demo and point to that from (2).

However my guess is that it still won't get merged until the QML gui itself is closer to readiness (as a replacement for the current GUI, or a standalone project). There's a general reluctance to merging code that isn't being actively used (yet).

@D33r-Gee
Copy link
Author

D33r-Gee commented Apr 29, 2025

@Sjors,

Thank you for your response and for clarifying the path forward.

I apologize for the confusion on my part regarding the intended repository for the interface change. Following your comment in QML PR #424, I incorrectly assumed you meant bitcoin-core/gui when you referred to "Bitcoin Core" rather than bitcoin/bitcoin. Thank you for clearing that up.

It's encouraging to hear that there's interest in potentially exposing the AssumeUTXO functionality to the QWidget GUI in the future. However, I remain committed to completing the implementation within the QML repository as the primary focus at this time.

To ensure I have the steps correct going forward: The QML PRs should first be merged into the bitcoin-core/gui-qml repository. After those are successfully integrated, I will then open the interface change PR (this one) against the bitcoin/bitcoin repository, linking back to the QML implementation for context. Is that correct?

@jarolrod
Copy link
Member

jarolrod commented Apr 29, 2025

@D33r-Gee as we discussed a while ago, this is part (ultimately) of the qml change-set, when qml is ready and it is being up-streamed, this feature, which you find useful as part of the qml change-set, would then be packaged up as part of the PR's integrating this into core.

This wouldn't be integrated into the qt widgets gui if qml is to replace it.

As is, this should be closed because the qt widgets gui can't use it, so this function goes to no-one.

@D33r-Gee
Copy link
Author

when qml is ready and it is being up-streamed, this feature, which you find useful as part of the qml change-set, would then be packaged up as part of the PR's integrating this into core.

Thank you for explaining the relationship between this interface change and the QML project so clearly. I see now that my timing was off, and this change is contingent upon the QML GUI being ready for upstreaming, not a standalone addition for the current GUI.

Just to ensure I've fully grasped the workflow: The steps involve getting the QML PRs merged into bitcoin-core/gui-qml first, and then the required interface changes for exposing the AssumeUTXO functionality will be introduced later in the bitcoin/bitcoin repository as part of the process to integrate the QML GUI itself. Is that a correct summary?

@jarolrod
Copy link
Member

@D33r-Gee no, your interface changes are part of what gets merged into the qml repo. It is part of the changeset to implement your assumeutxo work -> in the qml repo all in the qml repo. Then the process of packaging it up to upstream is, primarily, outside of your concern. I am just mentioning this to show why this shouldn't be open here.

@D33r-Gee
Copy link
Author

Thanks @Sjors and @jarolrod for your feedback.

After consideration I am opening a PR in this repo to expose the AssumeUTXO Load Snapshot functionality. It can be found in PR #870

@D33r-Gee D33r-Gee closed this Apr 30, 2025
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.

4 participants