-
Notifications
You must be signed in to change notification settings - Fork 41
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 (160,000 height) #424
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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> | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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())}; | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.