-
Notifications
You must be signed in to change notification settings - Fork 6
feat: bootstrap utxo module #484
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
- addresses, native assets, datums and reference scripts currently missing values from snapshot
- Extract UTXO-related types (SnapshotUTxO, SnapshotUTxOIdentifier, SnapshotAddress, SnapshotValue, SnapshotUTXOValue, UtxoEntry) from streaming_snapshot.rs into new utxo.rs module - Fix utxo_state module not receiving bootstrap UTXOs by adding default subscription to "cardano.snapshot" topic (was previously optional with no default, so subscription never happened) - Change publisher's tokio::spawn to blocking calls using block_in_place + Handle::current().block_on() to ensure messages are published before callbacks return - Add logging for UTXO batch publishing and receiving to aid debugging
# Conflicts: # modules/snapshot_bootstrapper/src/publisher.rs
|
Confirmation of this working |
buddhisthead
left a comment
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.
Wow, this looks great! So much nice cleanup and nice documentation about the issues.
Note: I initially did have a SnapshotCompleteMessage that marked the end of the boot-from-snapshot process, but found there is already a snapshot complete message. At that time, I didn't realize the order of mithril "snapshot" loading vs our boostrapping. It confused me for a while because there is the Cardano message that means "Mithril snapshot loading complete".
Description
This PR implements bootstrapping the UTxO state module from the streaming snapshot.
Related Issue(s)
Implements #386.
How was this tested?
Checklist
Impact / Side effects
Replaces the contents of the UtxoEntry struct with our own common types, so anything else relying on them will need updating. I'm not aware of anything that falls into this scope.
Reviewer notes / Areas to focus
I'm not sure if it's normal to have invalid utxos?
acropolis_module_utxo_state::state: slot=134956948 number=10844342 immutable_utxos=11234442 volatile_utxos=243 valid_utxos=11234421I'm not 100% sure about the address parsing, particularly the "other Shelley" bit where I've assumed that from_bytes_key() is the right thing for everything that isn't a stake address.
The flushing of any final utxo state message batch on completion of the snapshot could do with checking. My understanding from the very helpful comment is that the message sent by on_actions() blocks until all snapshot messages are dealt with, so I have added a call to flush any final batches just before that message is sent.
Sidenote: having a SnapshotComplete message sent on the snapshot channel might provide a cleaner place to hook this into. I think it also provides a simple mechanism for modules to drop their snapshot listeners should we wish to do that.