-
Notifications
You must be signed in to change notification settings - Fork 106
refactor(sync): split sync_state into sync_chain and sync_note_transport for granularity
#2091
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
Changes from all commits
7900e83
e240a90
ee50832
2128f79
81afbaf
066bf9e
eb087b6
7838054
4987e2e
20c4688
2d235d2
6561ac3
ac017f1
6ab4564
268c5c6
a1ead57
37c65e1
8ad4274
e212878
b22e62d
e34cc46
f0d1b94
b33b7f0
ab17474
b9ad816
57ea066
ccb3f4f
2e0d1d9
5fe0968
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 |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ | |
| //! let sync_summary: SyncSummary = client.sync_state().await?; | ||
| //! | ||
| //! println!("Synced up to block number: {}", sync_summary.block_num); | ||
| //! println!("New private notes: {}", sync_summary.new_private_notes.len()); | ||
| //! println!("Committed notes: {}", sync_summary.committed_notes.len()); | ||
| //! println!("Consumed notes: {}", sync_summary.consumed_notes.len()); | ||
| //! println!("Updated accounts: {}", sync_summary.updated_accounts.len()); | ||
|
|
@@ -100,37 +101,20 @@ where | |
| self.store.get_sync_height().await.map_err(Into::into) | ||
| } | ||
|
|
||
| /// Syncs the client's state with the current state of the Miden network and returns a | ||
| /// [`SyncSummary`] corresponding to the local state update. | ||
| /// Syncs the client's on-chain state with the current state of the Miden network and returns | ||
| /// a [`SyncSummary`] corresponding to the local state update. | ||
| /// | ||
| /// The sync process is done in multiple steps: | ||
| /// 1. A request is sent to the node to get the state updates. This request includes tracked | ||
| /// account IDs and the tags of notes that might have changed or that might be of interest to | ||
| /// the client. | ||
| /// 2. A response is received with the current state of the network. The response includes | ||
| /// information about new/committed/consumed notes, updated accounts, and committed | ||
| /// transactions. | ||
| /// 3. Tracked notes are updated with their new states. | ||
| /// 4. New notes are checked, and only relevant ones are stored. Relevant notes are those that | ||
| /// can be consumed by accounts the client is tracking (this is checked by the | ||
| /// [`crate::note::NoteScreener`]) | ||
| /// 5. Transactions are updated with their new states. | ||
| /// 6. Tracked public accounts are updated and private accounts are validated against the node | ||
| /// state. | ||
| /// 7. The MMR is updated with the new peaks and authentication nodes. | ||
| /// 8. All updates are applied to the store to be persisted. | ||
| pub async fn sync_state(&mut self) -> Result<SyncSummary, ClientError> { | ||
| /// Does **not** fetch private notes from the Note Transport Layer. Use | ||
| /// [`Client::sync_state`] for the combined sync, or call [`Client::sync_note_transport`] | ||
| /// separately. | ||
| /// | ||
| /// Builds the default sync input, runs [`StateSync::sync_state`] (see that method for the | ||
| /// detailed pipeline), applies the resulting update to the store, caches the partial MMR, and | ||
| /// prunes irrelevant blocks according to the configured cadence. | ||
| pub async fn sync_chain(&mut self) -> Result<SyncSummary, ClientError> { | ||
| self.ensure_genesis_in_place().await?; | ||
| self.ensure_rpc_limits_in_place().await?; | ||
|
|
||
| // Note Transport update | ||
| // TODO We can run both sync_state, fetch_transport_notes futures in parallel | ||
| if self.is_note_transport_enabled() { | ||
| let cursor = self.store.get_note_transport_cursor().await?; | ||
| let note_tags = self.store.get_unique_note_tags().await?; | ||
| self.fetch_transport_notes(cursor, note_tags).await?; | ||
| } | ||
|
|
||
| // Build sync state components | ||
| let note_screener = self.note_screener(); | ||
| let state_sync = StateSync::new( | ||
|
|
@@ -164,6 +148,36 @@ where | |
| Ok(sync_summary) | ||
| } | ||
|
|
||
| /// Fetches private notes from the Note Transport Layer for the tracked note tags. | ||
| /// | ||
| /// Returns the IDs of notes imported in this call. No-op (returns an empty vec) if note | ||
| /// transport is disabled. | ||
| pub async fn sync_note_transport(&mut self) -> Result<Vec<NoteId>, ClientError> { | ||
| if !self.is_note_transport_enabled() { | ||
| return Ok(Vec::new()); | ||
| } | ||
|
|
||
| let cursor = self.store.get_note_transport_cursor().await?; | ||
| let note_tags = self.store.get_unique_note_tags().await?; | ||
| self.fetch_transport_notes(cursor, note_tags).await | ||
| } | ||
|
|
||
| /// Runs the full client sync. | ||
| /// | ||
| /// First fetches private notes from the Note Transport Layer (see | ||
| /// [`Client::sync_note_transport`]), then syncs the client's on-chain state with the Miden | ||
| /// node (see [`Client::sync_chain`]). If note transport is disabled, this is equivalent to | ||
| /// [`Client::sync_chain`]. | ||
| /// | ||
| /// Fails fast on the first error. Private notes delivered via NTL are imported before the | ||
| /// chain sync reads its input set, so their nullifiers are checked in the same call. | ||
| pub async fn sync_state(&mut self) -> Result<SyncSummary, ClientError> { | ||
| let new_private_notes = self.sync_note_transport().await?; | ||
| let mut summary = self.sync_chain().await?; | ||
| summary.new_private_notes = new_private_notes; | ||
| Ok(summary) | ||
|
Comment on lines
+175
to
+178
Collaborator
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. nit: We could add something like |
||
| } | ||
|
JereSalo marked this conversation as resolved.
|
||
|
|
||
| /// Builds a default [`StateSyncInput`] from the current client state. | ||
| /// | ||
| /// This includes all tracked account headers, all unique note tags, all unspent input and | ||
|
|
@@ -307,6 +321,11 @@ pub struct SyncSummary { | |
| pub block_num: BlockNumber, | ||
| /// IDs of new public notes that the client has received. | ||
| pub new_public_notes: Vec<NoteId>, | ||
| /// IDs of private notes imported from the Note Transport Layer in this sync. | ||
| /// | ||
| /// Only populated by [`Client::sync_state`]; [`Client::sync_chain`] always leaves this empty | ||
| /// because it does not touch the Note Transport Layer. | ||
| pub new_private_notes: Vec<NoteId>, | ||
|
JereSalo marked this conversation as resolved.
|
||
| /// IDs of tracked notes that have been committed. | ||
| pub committed_notes: Vec<NoteId>, | ||
| /// IDs of notes that have been consumed. | ||
|
|
@@ -323,6 +342,7 @@ impl SyncSummary { | |
| pub fn new( | ||
| block_num: BlockNumber, | ||
| new_public_notes: Vec<NoteId>, | ||
| new_private_notes: Vec<NoteId>, | ||
| committed_notes: Vec<NoteId>, | ||
| consumed_notes: Vec<NoteId>, | ||
| updated_accounts: Vec<AccountId>, | ||
|
|
@@ -332,6 +352,7 @@ impl SyncSummary { | |
| Self { | ||
| block_num, | ||
| new_public_notes, | ||
| new_private_notes, | ||
| committed_notes, | ||
| consumed_notes, | ||
| updated_accounts, | ||
|
|
@@ -344,6 +365,7 @@ impl SyncSummary { | |
| Self { | ||
| block_num, | ||
| new_public_notes: vec![], | ||
| new_private_notes: vec![], | ||
| committed_notes: vec![], | ||
| consumed_notes: vec![], | ||
| updated_accounts: vec![], | ||
|
|
@@ -354,6 +376,7 @@ impl SyncSummary { | |
|
|
||
| pub fn is_empty(&self) -> bool { | ||
| self.new_public_notes.is_empty() | ||
| && self.new_private_notes.is_empty() | ||
| && self.committed_notes.is_empty() | ||
| && self.consumed_notes.is_empty() | ||
| && self.updated_accounts.is_empty() | ||
|
|
@@ -364,6 +387,7 @@ impl SyncSummary { | |
| pub fn combine_with(&mut self, mut other: Self) { | ||
| self.block_num = max(self.block_num, other.block_num); | ||
| self.new_public_notes.append(&mut other.new_public_notes); | ||
| self.new_private_notes.append(&mut other.new_private_notes); | ||
| self.committed_notes.append(&mut other.committed_notes); | ||
| self.consumed_notes.append(&mut other.consumed_notes); | ||
| self.updated_accounts.append(&mut other.updated_accounts); | ||
|
|
@@ -376,6 +400,7 @@ impl Serializable for SyncSummary { | |
| fn write_into<W: miden_tx::utils::serde::ByteWriter>(&self, target: &mut W) { | ||
| self.block_num.write_into(target); | ||
| self.new_public_notes.write_into(target); | ||
| self.new_private_notes.write_into(target); | ||
| self.committed_notes.write_into(target); | ||
| self.consumed_notes.write_into(target); | ||
| self.updated_accounts.write_into(target); | ||
|
|
@@ -390,6 +415,7 @@ impl Deserializable for SyncSummary { | |
| ) -> Result<Self, DeserializationError> { | ||
| let block_num = BlockNumber::read_from(source)?; | ||
| let new_public_notes = Vec::<NoteId>::read_from(source)?; | ||
| let new_private_notes = Vec::<NoteId>::read_from(source)?; | ||
| let committed_notes = Vec::<NoteId>::read_from(source)?; | ||
| let consumed_notes = Vec::<NoteId>::read_from(source)?; | ||
| let updated_accounts = Vec::<AccountId>::read_from(source)?; | ||
|
|
@@ -399,6 +425,7 @@ impl Deserializable for SyncSummary { | |
| Ok(Self { | ||
| block_num, | ||
| new_public_notes, | ||
| new_private_notes, | ||
| committed_notes, | ||
| consumed_notes, | ||
| updated_accounts, | ||
|
|
||
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.
This is a bit ambiguous: you can get private notes from the chain itself (or rather, we get the inclusion information for a private note that the client knew about), but I believe this just really displays the private notes retrieved from the NTL, right? If so, we'd need to make this clearer IMO.