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

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;
};

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

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?

re: #33117 (comment)

Thanks! I think b464f38 is an improvement over previous de11e3b 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 b464f38 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 b464f38, 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.

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.