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.

Sign up for GitHub

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 (160,000 height) #424

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

[DRAFT] WIP QML Load Snapshot Signet (160,000 height) #424

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc72124
qml: Introduce UI Flow for Loading Snapshot
D33r-Gee Oct 2, 2024
8beb50a
wip: Add minimal support for loading signet UTXO snapshots
D33r-Gee Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/interfaces/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,12 @@ class Node
//! List rpc commands.
virtual std::vector<std::string> listRpcCommands() = 0;

//! Load UTXO Snapshot.
virtual bool snapshotLoad(const std::string& path_string) = 0;

//! Get snapshot progress.
virtual double getSnapshotProgress() = 0;

//! Set RPC timer interface if unset.
virtual void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) = 0;

Expand Down
7 changes: 7 additions & 0 deletions src/kernel/chainparams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,13 @@ class SigNetParams : public CChainParams {

vFixedSeeds.clear();

m_assumeutxo_data = MapAssumeutxo{
{
160000,
Copy link
Member

Choose a reason for hiding this comment

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

8beb50a: the signet snapshot was merged into Bitcoin Core more than a year ago in bitcoin/bitcoin@edbed31. This also seems to be using the older serialization format from before bitcoin/bitcoin#29612.

I guess that's all fine for a proof of concept, but it would be better if the main branch of this repo was more up to date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Sjors! Thanks for taking a look and yes completely agree with:

it would be better if the main branch of this repo was more up to date

We are currently taking steps to sync with upstream, meanwhile I wanted to get the ball rolling, although I will pull in the update versions of the assume utxo functionality so that when the sync happens this PR can be merged with minimal refactoring.

Copy link
Member

Choose a reason for hiding this comment

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

Another thing you could do is look into making a PR to the Bitcoin Core repo with your proposed interface changes. One way to go about that is to somehow use them in existing or a new RPC method. E.g. snapshot load progress is probably useful to have available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another thing you could do is look into making a PR to the Bitcoin Core repo with your proposed interface changes.

That's the current medium term plan, once we have an implementation that works and makes sense here we plan to go upstream (bitcoin/bitcoin) and open a PR there.

{AssumeutxoHash{uint256S("0x5225141cb62dee63ab3be95f9b03d60801f264010b1816d4bd00618b2736e7be")}, 1278002},
},
};

base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,111);
base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,196);
base58Prefixes[SECRET_KEY] = std::vector<unsigned char>(1,239);
Expand Down
36 changes: 35 additions & 1 deletion src/node/interfaces.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <node/context.h>
#include <node/interface_ui.h>
#include <node/transaction.h>
#include <node/utxo_snapshot.h>
#include <policy/feerate.h>
#include <policy/fees.h>
#include <policy/policy.h>
Expand Down Expand Up @@ -395,9 +396,42 @@ class NodeImpl : public Node
{
m_context = context;
}
double getSnapshotProgress() override { return m_snapshot_progress.load(); }
bool snapshotLoad(const std::string& path_string) override
{
const fs::path path = fs::u8path(path_string);
if (!fs::exists(path)) { return false; }

AutoFile afile{fsbridge::fopen(path, "rb")};
if (afile.IsNull()) { return false; }

SnapshotMetadata metadata;
try {
afile >> metadata;
} catch (const std::exception& e) { return false; }

const uint256& base_blockhash = metadata.m_base_blockhash;

if (!m_context->chainman) { return false; }

ChainstateManager& chainman = *m_context->chainman;
CBlockIndex* snapshot_start_block = nullptr;

// Wait for the block to appear in the block index
//TODO: remove this once another method is implemented
constexpr int max_wait_seconds = 600; // 10 minutes
for (int i = 0; i < max_wait_seconds; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't have a blocking component here. If there are any issues we should just fail.

snapshot_start_block = WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(base_blockhash));
if (snapshot_start_block) break;
std::this_thread::sleep_for(std::chrono::seconds(1));
}

return chainman.ActivateSnapshot(afile, metadata, false);
}
ArgsManager& args() { return *Assert(Assert(m_context)->args); }
ChainstateManager& chainman() { return *Assert(m_context->chainman); }
NodeContext* m_context{nullptr};
std::atomic<double> m_snapshot_progress{0.0};
Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine the actual progress should be owned by the object in charge of doing the processing. ChainStateManager?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Will set it up so that the handler notifies the snapshot progress and that ownership belongs to ChainstateManager.

};

bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock, const CChain& active, const BlockManager& blockman)
Expand Down Expand Up @@ -510,7 +544,7 @@ class RpcHandlerImpl : public Handler
class ChainImpl : public Chain
{
public:
explicit ChainImpl(NodeContext& node) : m_node(node) {}
explicit ChainImpl(node::NodeContext& node) : m_node(node) {}
std::optional<int> getHeight() override
{
const int height{WITH_LOCK(::cs_main, return chainman().ActiveChain().Height())};
Expand Down
23 changes: 17 additions & 6 deletions src/qml/components/ConnectionSettings.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,21 @@ ColumnLayout {
id: root
signal next
signal gotoSnapshot
property bool snapshotImported: false
function setSnapshotImported(imported) {
snapshotImported = imported
property bool snapshotImportCompleted: false
property bool onboarding: false

Component.onCompleted: {
if (!onboarding) {
snapshotImportCompleted = chainModel.isSnapshotActive
} else {
snapshotImportCompleted = false
}
}

spacing: 4
Setting {
id: gotoSnapshot
visible: !root.onboarding
Layout.fillWidth: true
header: qsTr("Load snapshot")
description: qsTr("Instant use with background sync")
Expand All @@ -26,19 +34,22 @@ ColumnLayout {
height: 26
CaretRightIcon {
anchors.centerIn: parent
visible: !snapshotImported
visible: !snapshotImportCompleted
color: gotoSnapshot.stateColor
}
GreenCheckIcon {
anchors.centerIn: parent
visible: snapshotImported
visible: snapshotImportCompleted
color: Theme.color.transparent
size: 30
}
}
onClicked: root.gotoSnapshot()
}
Separator { Layout.fillWidth: true }
Separator {
visible: !root.onboarding
Layout.fillWidth: true
}
Setting {
Layout.fillWidth: true
header: qsTr("Enable listening")
Expand Down
94 changes: 45 additions & 49 deletions src/qml/components/SnapshotSettings.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,28 @@
import QtQuick 2.15
import QtQuick.Controls 2.15
import QtQuick.Layouts 1.15
import QtQuick.Dialogs 1.3

import "../controls"

ColumnLayout {
signal snapshotImportCompleted()
property int snapshotVerificationCycles: 0
property real snapshotVerificationProgress: 0
property bool snapshotVerified: false

id: columnLayout
signal back
property bool snapshotLoading: nodeModel.snapshotLoading
property bool snapshotLoaded: nodeModel.isSnapshotLoaded
property bool snapshotImportCompleted: chainModel.isSnapshotActive
property bool onboarding: false
property bool snapshotVerified: onboarding ? false : chainModel.isSnapshotActive
property string snapshotFileName: ""
property var snapshotInfo: ({})
property string selectedFile: ""

width: Math.min(parent.width, 450)
anchors.horizontalCenter: parent.horizontalCenter


Timer {
id: snapshotSimulationTimer
interval: 50 // Update every 50ms
running: false
repeat: true
onTriggered: {
if (snapshotVerificationProgress < 1) {
snapshotVerificationProgress += 0.01
} else {
snapshotVerificationCycles++
if (snapshotVerificationCycles < 1) {
snapshotVerificationProgress = 0
} else {
running = false
snapshotVerified = true
settingsStack.currentIndex = 2
}
}
}
}

StackLayout {
id: settingsStack
currentIndex: 0
currentIndex: onboarding ? 0 : snapshotLoaded ? 2 : snapshotVerified ? 2 : snapshotLoading ? 1 : 0

ColumnLayout {
Layout.alignment: Qt.AlignHCenter
Expand Down Expand Up @@ -77,11 +61,22 @@ ColumnLayout {
Layout.bottomMargin: 20
Layout.alignment: Qt.AlignCenter
text: qsTr("Choose snapshot file")
onClicked: {
settingsStack.currentIndex = 1
snapshotSimulationTimer.start()
onClicked: fileDialog.open()
}

FileDialog {
id: fileDialog
folder: shortcuts.home
selectMultiple: false
selectExisting: true
nameFilters: ["Snapshot files (*.dat)", "All files (*)"]
onAccepted: {
selectedFile = fileUrl.toString()
snapshotFileName = selectedFile
nodeModel.initializeSnapshot(true, snapshotFileName)
}
}
// TODO: Handle file error signal
}

ColumnLayout {
Expand All @@ -102,17 +97,10 @@ ColumnLayout {
Layout.leftMargin: 20
Layout.rightMargin: 20
header: qsTr("Loading Snapshot")
description: qsTr("This might take a while...")
}

ProgressIndicator {
id: progressIndicator
Layout.topMargin: 20
width: 200
height: 20
progress: snapshotVerificationProgress
Layout.alignment: Qt.AlignCenter
progressColor: Theme.color.blue
}
// TODO: add progress indicator once the snapshot progress is implemented
}

ColumnLayout {
Expand All @@ -137,20 +125,19 @@ ColumnLayout {
descriptionColor: Theme.color.neutral6
descriptionSize: 17
descriptionLineHeight: 1.1
description: qsTr("It contains transactions up to January 12, 2024. Newer transactions still need to be downloaded." +
" The data will be verified in the background.")
description: snapshotInfo && snapshotInfo["date"] ?
qsTr("It contains transactions up to %1. Newer transactions still need to be downloaded." +
Copy link
Member

@Sjors Sjors Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's not really "transactions up to", it's all coins (utxo set) that exist at the snapshot height.

Maybe say something like:

It contains a snapshot as of block %1. Once loaded, all blocks from %2 to the present are download. After that Bitcoin Core is ready to use, while all historical blocks from genesis to %2 are downloaded and validated in the background.

(intentionally leaves it vague what's in the snapshot)

Copy link
Contributor Author

@D33r-Gee D33r-Gee Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Great suggestion! I will run by the designers...

@yashrajd @GBKS what are your thoughts on the suggestion? Does it make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the sequence, this is the fourth snapshot-related screen (state) people see:

  1. Snapshot option in settings where people learn about the benefit of this feature
  2. "Load snapshot" screen that gives more detail about how it works
  3. "Loading snapshot" screen state with a loader that tells people they can do other stuff in the mean time
  4. "Snapshot loaded" screen state that informs people what was imported, what to keep in mind and what will happen next (this is the screen that we are talking about in this thread)

I think the point that not all historical transactions are in the snapshot is worth clarifying. The first time people are exposed to what this feature does is in 2, so I'd probably start tweaking at that point. We call it a "recent transaction snapshot" there, which is more precise than just "transactions" ("unspent transaction snapshot" would be a little closer).

We don't need to be 100% technically accurate and I'm not sure users are well-served if we're completely vague about this either. They should be able to form a good enough mental model for making decisions and understanding their impact.

So how about this?

It contains recent transactions up to %1. Newer and historical transactions will be automatically downloaded and verified.

Or this?

It contains unspent transactions up to %1. Newer and historical transactions will be automatically downloaded and verified.

Either way, we should ensure to tweak the text across the sequence if we make changes here.

We show the block height in a "View details" option below (did not check if it's implemented in this PR already), so not sure whether we need to include that in the text as well.

As for what happens next, my thinking was that we just say that the data will be filled in and verified here (keep it short), and then have indicators contextually in the UI as to what is happening (balance, send and transaction list have respective load states). So less absorption of logic here, and more contextual info in the appropriate places.

Sorry, this has gotten quite long. Hope it's a helpful response.

Copy link
Member

Choose a reason for hiding this comment

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

We don't need to be 100% technically accurate and I'm not sure users are well-served if we're completely vague about this either.

I think it's better to be incomplete than incorrect. The nice thing about being vague is that it's clear to the reader there's more to learn.

One reason the phrasing "recent / historical transactions" is potentially confusing is that people may think they can recover historical wallet transactions with it. But it has nothing to do with that.

The concept that's useful to explain is that the snapshot allows us to start syncing at block %1, saving time by skipping historical blocks. This way the user can get started (with their wallet) earlier. Meanwhile it's safe, because we do check those historical blocks in the background later.

Copy link

@yashrajd yashrajd Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

TL;DR I reviewed the onboarding text again, and it seems like the use of transactions terminology in the snapshot section provides context continuity with what's said before...see screenshot below.

Screenshot 2024-12-13 at 12 45 27

Unless we come up with something better, I'd be fine with retaining what we have currently though I agree it's inaccurate. Suggestions below:

I think it's better to be incomplete than incorrect. The nice thing about being vague is that it's clear to the reader there's more to learn.

It does seem like we can strike a better balance between technical accuracy & usefulness to user here, and use vagueness where required.

We show the block height in a "View details" option below (did not check if it's implemented in this PR already), so not sure whether we need to include that in the text as well.

Agree, this was in the initial mockups I did. edit: removed something here. read on...

The first time people are exposed to what this feature does is in 2, so I'd probably start tweaking at that point.

Agree.

  1. "Load snapshot" screen that gives more detail about how it works.

This screen can omit mention of transactions "The snapshot allows you to start using the application more quickly. Once loaded, it will be automatically verified in the background. Learn more" instead of "You can start using the application more quickly by loading a recent transaction snapshot. It will be automatically verified in the background."

  1. "Snapshot loaded" screen state that informs people what was imported, what to keep in mind and what will happen next (this is the screen that we are talking about in this thread)

I'd suggest something like "The loaded snapshot contains data up to [some date]. The initial download will continue now. The snapshot is verified in the background." instead of "It contains transactions up to September 10, 2023. Newer transactions still need to be downloaded. The data will be verified in the background."

Frame 10036

edit: rephrased my own suggestion slightly.

" The data will be verified in the background.").arg(snapshotInfo["date"]) :
qsTr("It contains transactions up to DEBUG. Newer transactions still need to be downloaded." +
" The data will be verified in the background.")
}

ContinueButton {
Layout.preferredWidth: Math.min(300, columnLayout.width - 2 * Layout.leftMargin)
Layout.topMargin: 40
Layout.alignment: Qt.AlignCenter
text: qsTr("Done")
onClicked: {
snapshotImportCompleted()
connectionSwipe.decrementCurrentIndex()
connectionSwipe.decrementCurrentIndex()
}
onClicked: back()
}

Setting {
Expand Down Expand Up @@ -188,16 +175,25 @@ ColumnLayout {
font.pixelSize: 14
}
CoreText {
text: qsTr("200,000")
text: snapshotInfo && snapshotInfo["height"] ?
snapshotInfo["height"] : qsTr("DEBUG")
Layout.alignment: Qt.AlignRight
font.pixelSize: 14
}
}
Separator { Layout.fillWidth: true }
CoreText {
text: qsTr("Hash: 0x1234567890abcdef...")
text: snapshotInfo && snapshotInfo["hashSerialized"] ?
qsTr("Hash: %1").arg(snapshotInfo["hashSerialized"].substring(0, 13) + "...") :
qsTr("Hash: DEBUG")
font.pixelSize: 14
}

Component.onCompleted: {
if (snapshotVerified || snapshotLoaded) {
snapshotInfo = chainModel.getSnapshotInfo()
}
}
}
}
}
Expand Down
22 changes: 22 additions & 0 deletions src/qml/models/chainmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@
#include <QThread>
#include <QTime>
#include <interfaces/chain.h>
#include <node/utxo_snapshot.h>
#include <kernel/chainparams.h>
#include <validation.h>

ChainModel::ChainModel(interfaces::Chain& chain)
: m_chain{chain}
// m_params{Params()}
{
QTimer* timer = new QTimer();
connect(timer, &QTimer::timeout, this, &ChainModel::setCurrentTimeRatio);
Expand Down Expand Up @@ -101,3 +105,21 @@ void ChainModel::setCurrentTimeRatio()

Q_EMIT timeRatioListChanged();
}

// Using hardcoded snapshot info to display in SnapshotSettings.qml
QVariantMap ChainModel::getSnapshotInfo() {
QVariantMap snapshot_info;

const MapAssumeutxo& valid_assumeutxos_map = Params().Assumeutxo();
if (!valid_assumeutxos_map.empty()) {
const int height = valid_assumeutxos_map.rbegin()->first;
const auto& hash_serialized = valid_assumeutxos_map.rbegin()->second.hash_serialized;
int64_t date = m_chain.getBlockTime(height);

snapshot_info["height"] = height;
snapshot_info["hashSerialized"] = QString::fromStdString(hash_serialized.ToString());
snapshot_info["date"] = QDateTime::fromSecsSinceEpoch(date).toString("MMMM d yyyy");
}

return snapshot_info;
}
6 changes: 5 additions & 1 deletion src/qml/models/chainmodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class ChainModel : public QObject
Q_PROPERTY(quint64 assumedBlockchainSize READ assumedBlockchainSize CONSTANT)
Q_PROPERTY(quint64 assumedChainstateSize READ assumedChainstateSize CONSTANT)
Q_PROPERTY(QVariantList timeRatioList READ timeRatioList NOTIFY timeRatioListChanged)
Q_PROPERTY(bool isSnapshotActive READ isSnapshotActive NOTIFY isSnapshotActiveChanged)

public:
explicit ChainModel(interfaces::Chain& chain);
Expand All @@ -36,18 +37,21 @@ class ChainModel : public QObject
quint64 assumedBlockchainSize() const { return m_assumed_blockchain_size; };
quint64 assumedChainstateSize() const { return m_assumed_chainstate_size; };
QVariantList timeRatioList() const { return m_time_ratio_list; };

bool isSnapshotActive() const { return m_chain.hasAssumedValidChain(); };
int timestampAtMeridian();

void setCurrentTimeRatio();

Q_INVOKABLE QVariantMap getSnapshotInfo();

public Q_SLOTS:
void setTimeRatioList(int new_time);
void setTimeRatioListInitial();

Q_SIGNALS:
void timeRatioListChanged();
void currentNetworkNameChanged();
void isSnapshotActiveChanged();

private:
QString m_current_network_name;
Expand Down
Loading
Loading