D33r-Gee commented at 3:31 PM on August 1, 2025: none

Add UTXO Snapshot Loading Interface and Progress Notifications

This PR implements a complete interface for loading UTXO snapshots through the GUI, including progress notifications to provide user feedback during the loading process.

Changes

Commit 1: interfaces: expose UTXO snapshot functionality (loading/activate) in node interface

Added a new virtual method snapshot to the node interface for loading UTXO snapshots from a path.
Added snapshot.h to src/interfaces defining the Snapshot interface; activate() returns util::Result<const CBlockIndex*> to carry either the base block index on success or a descriptive error message on failure, matching the shape of ChainstateManager::ActivateSnapshot().
Added a getMetadata accessor so callers can read snapshot metadata populated during activation without re-parsing the file.
Added SnapshotImpl in src/node/interfaces.cpp: opens the snapshot file, parses metadata, and forwards to ChainstateManager::ActivateSnapshot(), propagating any error as util::Error{...}.

This change enhances the user's ability to load UTXO snapshots through the GUI and/or RPC.

Commit 2: `notifications: Implement snapshot load progress notifications`

Added a new signal SnapshotLoadProgress for snapshot load progress in the UI interface.
Updated the notifications interface to include a method for reporting snapshot loading progress.
Enhanced the ChainstateManager to report progress during snapshot loading.
Implemented the necessary handler in the kernel notifications to relay progress updates.

This change improves user feedback during the snapshot loading process.

Commit 3: `rpc: refactor loadtxoutset to use snapshot interface`

Updated the loadtxoutset RPC to call the new snapshot interface instead of directly working with chainstate internals. File opening, metadata parsing, and activation now all happen inside interfaces::Snapshot::activate() rather than being split between the RPC handler and the interface.
Removed the redundant pre-parse block in the RPC handler; its file-open and metadata-parse work duplicated what activate() already performs.
Collapsed the RPC error handling to a single RPC_INTERNAL_ERROR built from util::ErrorString(activation_result).original, using the "Unable to load UTXO snapshot: %s. (%s)" format established in #30267 / #30395.
Read coins_loaded from snapshot->getMetadata() after activate() populates it.
Updated feature_assumeutxo.py to assert on the unified error code for file-open and metadata-parse failures (message substrings unchanged). Improves modularity by consolidating snapshot handling through the interface layer.

Technical Details

The implementation includes:

UI Interface Enhancement Added SnapshotLoadProgress signal to CClientUIInterface with corresponding signal declaration and implementation.
Node Interface Added loadSnapshot virtual method to the node interface with implementation in interfaces.cpp.
Progress Notifications Implemented snapshotLoadProgress method in KernelNotifications class to relay progress updates to the UI.
Base Interface Added snapshotLoadProgress virtual method to the kernel notifications interface.

Benefits

User Experience Provides real-time feedback during UTXO snapshot loading operations.
GUI Integration Enables loading UTXO snapshots directly through the Bitcoin Core GUI.
Progress Tracking Users can monitor the progress of snapshot loading operations.
Consistent Interface Follows the existing pattern for progress notifications in the codebase.

Testing

Daemon build and launch bitcoind then with bitcoin-cli load a snapshot using:

./bitcoin-cli -rpcclienttimeout=0 -signet loadtxoutset /path/to/utxo-snapsshot.dat

QT Widgets: build and run this draft PR in legacy gui repo
QML version: for a modern look build and run this PR in the newly updated gui-qml repo

DrahtBot added the label interfaces on Aug 1, 2025

DrahtBot commented at 3:31 PM on August 1, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33117.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Approach ACK	sedited
Stale ACK	pinheadmz

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35105 (Refactor: Updated TransactionError to use util::Expected and removed TransactionError:OK by kevkevinpal)
#35043 (refactor: Properly return from ThreadSafeQuestion signal + btcsignals follow-ups by maflcko)
#34806 (refactor: logging: Various API improvements by ajtowns)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

D33r-Gee referenced this in commit 26a2e0fccb on Aug 1, 2025

D33r-Gee referenced this in commit 5e561eff75 on Aug 1, 2025

Sjors commented at 6:41 PM on August 1, 2025: member

I plan to test this soon(tm).

D33r-Gee commented at 8:54 PM on August 1, 2025: none

I plan to test this soon(tm).

Thanks @Sjors !

in src/interfaces/node.h:209 in de11e3b89f outdated

 204 | @@ -204,6 +205,9 @@ class Node
 205 |      //! List rpc commands.
 206 |      virtual std::vector<std::string> listRpcCommands() = 0;
 207 |  
 208 | +    //! Load UTXO Snapshot.
 209 | +    virtual bool loadSnapshot(AutoFile& afile, const node::SnapshotMetadata& metadata, bool in_memory) = 0;

ryanofsky commented at 10:00 PM on August 1, 2025:

In commit "interfaces: expose UTXO snapshot loading in node interface" (de11e3b89f8aba9a90a3419086807e5fee0f941d)

Note with #10102 if the GUI & node are running in separate processes, it could be a awkward for the gui process to create an AutoFile object and pass it to the node like this. Technically it could probably work: the AutoFile could be passed as a file descriptor or a (path, position) tuple. But it doesn't seem like a clean separation of responsibilities if the GUI process is reading the beginning of the snapshot file and the node process is reading the rest of the file.

Probably current interface is ok, but it might be more ideal if node provided a more generic loadSnapshot method and the GUI didn't directly access the file:

class Node
{
    // ...
    std::unique_ptr<Snapshot> loadSnapshot(const fs::path& path) = 0;
    // ...
};

class Snapshot
{
public:
    virtual ~Snapshot() = default;
    node::SnapshotMetadata getMetadata() = 0;
    bool activate() = 0;
};

D33r-Gee commented at 11:35 PM on August 1, 2025:

Thanks for the review... will update soon addressing this

D33r-Gee commented at 4:24 PM on August 4, 2025:

with e79c50c updated the node.h and added snapshot.h to the /interfaces. Also updated interfaces.cpp with a SnapshotImpl class to process the path and extracts the metada and activate the snapshot. @ryanofsky let me know if that's what you had in mind?

ryanofsky commented at 4:46 PM on August 4, 2025:

re: #33117 (review)

Thanks! I think b464f388197c5c31937d309d817d38c4b4849a91 is an improvement over previous de11e3b89f8aba9a90a3419086807e5fee0f941d because it passes an fs::path argument from the GUI to the node instead of an AutoFile file argument. This is nice because it lets node be fully responsible for parsing the snapshot file instead of relying on the GUI.

One comment on b464f388197c5c31937d309d817d38c4b4849a91 is that it looks like the Snapshot class is not actually necessary in its current form. The reason I suggested adding a Snapshot class above was that I thought GUI wanted to display SnapshotMetadata information before activating the snapshot, and a Snapshot class could provide separate getMetadata() and activate() methods to allow that. But since the Snapshot class only has a single activate() method in b464f388197c5c31937d309d817d38c4b4849a91, it looks like the class is unnecessary, and you could just have a plain Node::activateSnapshot(fs::path) method that returns bool instead of a Snapshot.

Any approach is fine though, so you should choose whatever seems to make the most sense.

D33r-Gee commented at 3:23 PM on August 6, 2025:

Thanks for taking a look! RE:

The reason I suggested adding a Snapshot class above was that I thought GUI wanted to display SnapshotMetadata information before activating the snapshot, and a Snapshot class could provide separate getMetadata()

I took some time to test various scenarios downstream in the gui-qml project. While we aren't directly utilizing the snapshot data at the moment, I believe your initial instincts about introducing a Snapshot class were well-founded. This abstraction will become increasingly valuable as we begin to expose the dump/generate snapshot functionalities trough the GUI.

D33r-Gee force-pushed on Aug 4, 2025

pinheadmz commented at 2:37 PM on August 5, 2025: member

concept ACK, tested the mainnet snapshot load with the QML gui branch built on top of this

fanquake commented at 1:09 PM on August 6, 2025: member

https://github.com/bitcoin/bitcoin/actions/runs/16728433539/job/47504424705?pr=33117#step:10:445:

D:\a\bitcoin\bitcoin\src\node\interfaces.cpp(115,40): error C2220: the following warning is treated as an error [D:\a\bitcoin\bitcoin\build\src\bitcoin_node.vcxproj]
D:\a\bitcoin\bitcoin\src\node\interfaces.cpp(115,40): warning C4101: 'e': unreferenced local variable [D:\a\bitcoin\bitcoin\build\src\bitcoin_node.vcxproj]

D33r-Gee force-pushed on Aug 6, 2025

D33r-Gee commented at 4:48 PM on August 7, 2025: none

with 8bc4035 it has been rebased and the fuzzing CI error has been addressed

in src/node/interfaces.cpp:119 in 3c8b578836 outdated

 114 | +            afile >> metadata;
 115 | +        } catch (const std::exception&) {
 116 | +            return false;
 117 | +        }
 118 | +
 119 | +        auto activation_result{m_chainman.ActivateSnapshot(afile, metadata, m_in_memory)};

Sjors commented at 7:52 AM on September 8, 2025:

In 3c8b578836dd0d8438a8aac81a1166df84811562 interfaces: expose UTXO snapshot loading in node interface: a good way to add test coverage to this new interface would be to use it in the loadtxoutset RPC.

For an example, see the various uses of EnsureMining(node);

D33r-Gee commented at 4:31 PM on September 8, 2025:

Thanks @Sjors for providing feedback and suggesting to add test coverage via the RPC...

Looking into it now and will update shortly

D33r-Gee commented at 4:36 PM on September 15, 2025:

a good way to add test coverage to this

refactored loadtxoutset to utilize the new snapshot interface...

Sjors commented at 7:57 AM on September 8, 2025: member

At first glance this looks very reasonable. I also tested it in the (current) GUI with https://github.com/bitcoin-core/gui/pull/870

I had a similar concern as @ryanofsky about how to go about passing the snapshot file (path) over the interface. The current approach in 3c8b578836dd0d8438a8aac81a1166df84811562 is the same as how we handle wallet backups (restoreWallet), so probably fine?

sedited commented at 9:54 AM on September 8, 2025: contributor

Approach ACK

D33r-Gee force-pushed on Sep 15, 2025

D33r-Gee commented at 4:34 PM on September 15, 2025: none

With 30d9bdb, the loadtxoutset RPC command has been refactored to use the snapshot interface, as suggested by @Sjors. Additionally, the interface now includes getMetadata and getActivationResult methods to ensure parity with the legacy RPC command.

D33r-Gee force-pushed on Sep 15, 2025

D33r-Gee commented at 10:13 PM on September 15, 2025: none

with 94d68d1 RPC Error handling has been restored through the interface

D33r-Gee force-pushed on Sep 16, 2025

DrahtBot added the label Needs rebase on Oct 24, 2025

D33r-Gee force-pushed on Nov 19, 2025

D33r-Gee commented at 6:25 PM on November 19, 2025: none

with 36ae2ef rebased over master.

No changes to commits...

DrahtBot removed the label Needs rebase on Nov 19, 2025

D33r-Gee force-pushed on Nov 19, 2025

D33r-Gee commented at 11:24 PM on November 19, 2025: none

with d0c32c2 fixed RPC command issue that surfaced after rebase

DrahtBot added the label CI failed on Dec 15, 2025

DrahtBot closed this on Dec 16, 2025

DrahtBot reopened this on Dec 16, 2025

DrahtBot removed the label CI failed on Dec 16, 2025

in src/validation.cpp:5968 in e8aa3a6d43 outdated

5962 | @@ -5963,6 +5963,10 @@ util::Result<void> ChainstateManager::PopulateAndValidateSnapshot(
5963 |                  --coins_left;
5964 |                  ++coins_processed;
5965 |  
5966 | +                // Show Snapshot Loading progress
5967 | +                double progress = static_cast<double>(coins_processed) / static_cast<double>(coins_count);
5968 | +                GetNotifications().snapshotLoadProgress(progress);

pinheadmz commented at 7:23 PM on January 20, 2026:

e8aa3a6d43760ffd76c247211f769624a48c8828

This will fire 100 million times during a mainnet snapshot load. Is that overkill? The log messages for example are reduced by 100x at least.

D33r-Gee commented at 10:18 PM on January 26, 2026:

Thanks for reviewing the code... Looking into it now... Will respond shortly. Got some ideas, testing them downstream first (qt and gui-qml)

D33r-Gee commented at 6:19 PM on February 25, 2026:

with 8c932ff addressed the snapshot load progress notification to batch progress updates for every 100,000 coins processed (and upon completion).

It was tested downstream in the qt and qml)

pinheadmz commented at 8:32 PM on January 20, 2026: member

code review ACK at d0c32c28e5

The new interface and signal handlers are fairly straight-forward and follow existing patterns. Consuming the new interface in the loadtxoutset RPC ensures it is covered by tests. Corecheck indicates some of the failure cases are not covered by tests, I want to look at that a little closer.

I also plan on going through the flow with both GUIs as well. I think this is an important feature to expose to GUI users, and this is the right way to get there.

Only one question below so far

DrahtBot requested review from sedited on Jan 20, 2026

D33r-Gee force-pushed on Feb 25, 2026

D33r-Gee commented at 6:20 PM on February 25, 2026: none

With 8c932ff addressed @pinheadmz comment about the loading progress notifications also it has been rebased over master

sedited requested review from pinheadmz on Mar 16, 2026

sedited requested review from stickies-v on Mar 25, 2026

DrahtBot added the label Needs rebase on Mar 30, 2026

D33r-Gee force-pushed on Mar 31, 2026

D33r-Gee commented at 4:30 PM on March 31, 2026: none

with 34e536d rebased over master

DrahtBot removed the label Needs rebase on Mar 31, 2026

DrahtBot added the label Needs rebase on Apr 9, 2026

D33r-Gee force-pushed on Apr 9, 2026

D33r-Gee commented at 7:36 PM on April 9, 2026: none

with 9dbfdf8 it has been rebased over master

DrahtBot removed the label Needs rebase on Apr 9, 2026

in src/interfaces/snapshot.h:1 in 740add96ec

   0 | @@ -0,0 +1,40 @@
   1 | +// Copyright (c) 2024 The Bitcoin Core developers

pinheadmz commented at 5:22 PM on April 10, 2026:

740add96ec6149174d69b05cab1088dd7f7dc776

nit: can remove the year Copyright (c) The Bitcoin Core developers

in src/node/interfaces.cpp:151 in 740add96ec outdated

 146 | +    {
 147 | +        if (m_activation_result.has_value()) {
 148 | +            return m_activation_result;
 149 | +        }
 150 | +        return std::nullopt;
 151 | +    }

pinheadmz commented at 5:29 PM on April 10, 2026:

740add96ec6149174d69b05cab1088dd7f7dc776

Since m_activation_result is already an optional I think you can shortcut the logic here?

std::optional<const CBlockIndex*> getActivationResult() const override
{
    return m_activation_result;
}

in src/rpc/blockchain.cpp:3429 in 9dbfdf81ca

3425 | +        if (error_msg.find("Unable to parse metadata") != std::string::npos) {
3426 | +            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, error_msg);
3427 | +        }
3428 | +        if (error_msg.find("Snapshot file does not exist") != std::string::npos) {
3429 | +            throw JSONRPCError(RPC_INVALID_PARAMETER, "Couldn't open file " + path.utf8string() + " for reading.");
3430 | +        }

pinheadmz commented at 5:42 PM on April 10, 2026:

9dbfdf81caff7cab40db94be9553601eacffd63c

Seems a little awkward to parse error strings for the sake of RPC errors. I think the generic one you have below on L3431 is fine, and if you want to include the failed path in the error message, it could happen over in class SnapshotImpl where the strings are set.

in src/rpc/blockchain.cpp:3436 in 9dbfdf81ca

3440 |  
3441 | -    SnapshotMetadata metadata{chainman.GetParams().MessageStart()};
3442 | +    const node::SnapshotMetadata* metadata;
3443 |      try {
3444 | -        afile >> metadata;
3445 | +        metadata = &snapshot->getMetadata();

pinheadmz commented at 5:44 PM on April 10, 2026:

9dbfdf81caff7cab40db94be9553601eacffd63c

Would this ever throw? Probably don't need the try/catch

in src/rpc/blockchain.cpp:3442 in 9dbfdf81ca

3447 |          throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("Unable to parse metadata: %s", e.what()));
3448 |      }
3449 |  
3450 | -    auto activation_result{chainman.ActivateSnapshot(afile, metadata, false)};
3451 | +    auto activation_result = snapshot->getActivationResult();
3452 |      if (!activation_result) {

pinheadmz commented at 5:49 PM on April 10, 2026:

9dbfdf81caff7cab40db94be9553601eacffd63c

The snapshot is activated (or not) up on line 3419. Will this call to getActivationResult() ever be useful after that?

pinheadmz commented at 5:50 PM on April 10, 2026: member

code review 9dbfdf81ca

h/t Claude !

D33r-Gee force-pushed on Apr 13, 2026

D33r-Gee commented at 7:11 PM on April 13, 2026: none

with 9dab30a addressed @pinheadmz comments. Thanks for reviewing!

interfaces: expose UTXO snapshot functionality (loading/activate) in node interface

- Added a new virtual method `snapshot` to the node interface for loading UTXO snapshots.
- Added `snapshot.h` to src/interfaces defining the Snapshot interface. `activate()`
  returns `util::Result<const CBlockIndex*>` to carry either the base block index
  on success or a descriptive error message on failure, matching the shape of
  `ChainstateManager::ActivateSnapshot()`.
- Added a `getMetadata` accessor so callers can read snapshot metadata populated
  during activation without re-parsing the file.
- Added `SnapshotImpl` in src/node/interfaces.cpp: opens the snapshot file,
  parses metadata, and forwards to `ChainstateManager::ActivateSnapshot()`,
  propagating any error as `util::Error{...}`.

This change enhances the user ability to load utxo snapshots through the GUI and/or RPC.

Made-with: Cursor

7406a3efa1

notifications: Implement snapshot load progress notifications

- Added a new signal for snapshot load progress in the UI interface.
- Updated the notifications interface to include a method for reporting snapshot loading progress.
- Enhanced the ChainstateManager to report progress during snapshot loading.
- Implemented the necessary handler in the kernel notifications to relay progress updates.

This change improves user feedback during the snapshot loading process.

4db862ff06

rpc: Refactor loadtxoutset to utilize new node interface for UTXO snapshot loading

- Updated the loadtxoutset function to use the new node interface for loading
  UTXO snapshots. File opening, metadata parsing, and activation now all happen
  inside `interfaces::Snapshot::activate()` rather than being split between the
  RPC handler and the interface.
- Replaced direct file handling with snapshot activation through the node
  interface. The previous pre-parse block in the RPC handler was redundant
  with the equivalent work already performed inside `activate()`.
- Collapsed the RPC error handling to a single `RPC_INTERNAL_ERROR` built from
  `util::ErrorString(activation_result).original`, matching the convention
  established in #30267 / #30395.
- Enhanced the result object to correctly reference metadata from the snapshot
  via `snapshot->getMetadata()`.
- Updated `feature_assumeutxo.py` to assert on the unified error code for
  file-open and metadata-parse failures.

This change streamlines the UTXO snapshot loading process and leverages the new interface capabilities.

Made-with: Cursor

328fef6bae

in src/rpc/blockchain.cpp:3426 in 9dab30ad3d

3429 | +        }
3430 | +        try {
3431 | +            afile >> metadata;
3432 | +        } catch (const std::ios_base::failure& e) {
3433 | +            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("Unable to parse metadata: %s", e.what()));
3434 | +        }

pinheadmz commented at 7:31 PM on April 17, 2026:

I don't understand whats happening here. If the file can't be opened for reading, or metadata can't be parsed, won't that come up in activate() on line 3433?

D33r-Gee commented at 6:41 PM on April 21, 2026:

I don't understand whats happening here. If the file can't be opened for reading, or metadata can't be parsed, won't that come up in activate() on line 3433?

Thanks for reviewing @pinheadmz and good catch! This led me to revise the approach with the help of Claude.

You are correct activate() already opens the file and parses metadata internally, so doing it again here was redundant. The only reason these specific RPC_INVALID_PARAMETER / RPC_DESERIALIZATION_ERROR codes were being produced at the RPC layer was to retrofit finer error classification on top of the old bool activate() signature.

D33r-Gee force-pushed on Apr 21, 2026

D33r-Gee commented at 6:56 PM on April 21, 2026: none

with 328fef6 revisited the approach since @pinheadmz exposed redundancy that was already addressed by #30267 and later tighten up with #30395

ChainstateManager::ActivateSnapshot still returns util::Result<CBlockIndex*> on master today, but my original approach on this branch's interfaces::Snapshot wrapper was unwrapping that and reducing it back to a bool + getLastError() + getActivationResult() triad, which is why I ended up reintroducing the pre-parse block just to recover error classification.

Here are the new changes introduced:

7406a3efa1cd8b86e621f0af63ea41fbd94b83ff interfaces: expose UTXO snapshot functionality ...

interfaces::Snapshot::activate() now returns util::Result<const CBlockIndex*> directly, carrying either the snapshot base block index on success or a descriptive error message on failure.

getActivationResult() and getLastError() are gone — the result carries both. SnapshotImpl correspondingly loses its m_activation_result / m_last_error members and just forwards ChainstateManager::ActivateSnapshot's result.

328fef6bae4007cb3903c6d2b5653f91c9bf14be rpc: Refactor loadtxoutset ...

The pre-parse block is deleted. The RPC handler calls activate() once and surfaces any failure via a single RPC_INTERNAL_ERROR with util::ErrorString(activation_result).original, using the same "Unable to load UTXO snapshot: %s. (%s)" format merged in #30395.

coins_loaded is now read from snapshot->getMetadata() after activate() populates it.

feature_assumeutxo.py is updated in the same commit: since assert_raises_rpc_error is substring-based, only the error codes for file-open (-8 → -32603) and metadata-parse (-22 → -32603) had to change; the message assertions still match unchanged.

Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications #33117

Add UTXO Snapshot Loading Interface and Progress Notifications

Changes

Commit 1: interfaces: expose UTXO snapshot functionality (loading/activate) in node interface

Commit 2: notifications: Implement snapshot load progress notifications

Commit 3: rpc: refactor loadtxoutset to use snapshot interface

Technical Details

Benefits

Testing

Code Coverage & Benchmarks

Reviews

Conflicts

Commit 2: `notifications: Implement snapshot load progress notifications`

Commit 3: `rpc: refactor loadtxoutset to use snapshot interface`