Conversation
MicaiahReid
left a comment
There was a problem hiding this comment.
I think a little more context would help me understand some of the whys! Plus one or two hygiene comments
crates/core/src/rpc/ws.rs
Outdated
| let snapshot_id = format!("snapshot_{}", chrono::Utc::now().timestamp_nanos_opt().unwrap_or(0)); | ||
|
|
||
| // Send initial notification that snapshot import has started | ||
| let start_notification = crate::surfnet::SnapshotImportNotification { |
There was a problem hiding this comment.
Hmm, it seems fishy that we're sending this hear as well as in the actual subscribe_for_snapshot_import_updates function - it should just be in the one in the svm file, right?
crates/core/src/surfnet/svm.rs
Outdated
|
|
||
| // TODO: The actual loading should be done via the load_snapshot method | ||
| // For now, we simulate the process and send completion | ||
| tokio::time::sleep(tokio::time::Duration::from_millis(1000)).await; |
There was a problem hiding this comment.
I think the load_snapshot fn should now exist! And I believe it's synchronous so that's nice.
crates/core/src/surfnet/svm.rs
Outdated
| async fn fetch_snapshot_from_url( | ||
| snapshot_url: &str, | ||
| ) -> Result<std::collections::BTreeMap<String, Option<surfpool_types::AccountSnapshot>>, Box<dyn std::error::Error + Send + Sync>> { | ||
| // For now, we'll use reqwest to fetch the snapshot data |
There was a problem hiding this comment.
In the future what will this be?
| subscribe, | ||
| name = "snapshotSubscribe" | ||
| )] | ||
| fn snapshot_subscribe( |
There was a problem hiding this comment.
This seems like a weird name, cause it's not just subscribing to notifications about a snapshot being imported, it's actually initiating the import.
Also, why are we having surfpool fetch snapshots instead of just receiving them via some RPC method?
No description provided.