feat: make `SyncNotes` return multiple blocks by TomasArrachea · Pull Request #1843 · 0xMiden/node

TomasArrachea · 2026-03-25T22:08:11Z

Closes #1809

This PR updates the RPC component to batch multiple blocks into a single SyncNotes response.

Changes

RPC sync_notes handler now loops over store calls, accumulating blocks until the 4MB budget is exceeded or the range is exhausted
Proto: changed SyncNotesResponse proto from single-block (header + mmr_path + notes) to multi-block (repeated NoteSyncBlock blocks + BlockRange). Added StoreSyncNotesResponse for the store-internal single-block response, using BlockRange instead of PaginationInfo.
Store's sync_notes uses open_at(block_num, checkpoint) instead of open(block_num) so MMR proofs are relative to block_to

Follow ups:

Instead of batching the sync note responses at the RPC component, batch them on the store and minimize the gRPC requests.
Deduplicate the authentication nodes between returned note updates.

proto/proto/rpc.proto

bobbinth

Looks good! Thank you! I left a few small comments inline.

bobbinth · 2026-03-27T02:47:07Z

proto/proto/rpc.proto

+    BlockRange block_range = 5;

-    // List of all notes together with the Merkle paths from `response.block_header.note_root`.
-    repeated note.NoteSyncRecord notes = 4;
+    // Blocks containing matching notes, ordered by block number ascending.
+    // May be empty if no notes matched in the range.
+    repeated NoteSyncBlock blocks = 6;


The field numbering looks a bit off here. Could block_range not start at 1?

bobbinth · 2026-03-27T02:58:49Z

crates/store/src/state/sync_state.rs

+/// Estimated byte size of a single [`NoteSyncRecord`].
+///
+/// Note ID (~38 bytes) + index + metadata (~26 bytes) + sparse merkle path with 16
+/// siblings (~608 bytes).
+const NOTE_RECORD_BYTES: usize = 700;


This is fine for now, but most likely will be a significant overestimate because sparse Merkle paths get compressed, and in most cases shouldn't be more than a couple hundred bytes. But the compression depends on how many paths there are (the more paths, the worse the compression) - so, taking the worst case is fine for now.

bobbinth · 2026-03-27T03:02:53Z

crates/store/src/state/sync_state.rs

+/// Estimated byte size of a [`NoteSyncBlock`] excluding its notes.
+///
+/// `BlockHeader` (~341 bytes) + MMR proof with 32 siblings (~1216 bytes).
+const BLOCK_OVERHEAD_BYTES: usize = 1600;


Not for this PR, but it would be good to make this based on the actual serialization sizes. For example, we could have BlockHeader::SERIALIZED_SIZE constant and use it here.

Let's create an issue for this (and include NOTE_RECORD_BYTES in this issue too).

igamigo

Looks great! I think there are a couple of edge cases that might need addressing though.

igamigo · 2026-03-27T04:22:06Z

proto/proto/rpc.proto

+    // `block_to` matches the request's `block_range.block_to`, or the chain tip if it was
+    // not specified.


nit: This is not always true, right? The text below mentions that the block_to may be smaller than what the user requested (and the chain tip as well)

igamigo · 2026-03-27T04:24:13Z

crates/store/src/state/sync_state.rs

-    /// - `note_tags`: The tags the client is interested in, resulting notes are restricted to the
-    ///   first block containing a matching note.
-    /// - `block_range`: The range of blocks from which to synchronize notes.
+    /// Returns as many blocks with matching notes as fit within the response payload


nit: You can reference the max payload size constant here

igamigo · 2026-03-27T04:31:31Z

crates/store/src/server/rpc_api.rs

@@ -130,26 +130,34 @@ impl rpc_server::Rpc for StoreApi {
        let request = request.into_inner();

        let chain_tip = self.state.latest_block_num().await;


nit (feel free to disregard): There's no need to read the latest block number if the user set a block_to

igamigo · 2026-03-27T04:32:25Z

crates/store/src/server/rpc_api.rs

        let request = request.into_inner();

        let chain_tip = self.state.latest_block_num().await;
+        let requested_block_to = request.block_range.as_ref().and_then(|r| r.block_to);


nit: I would move this down to be below line 134

igamigo · 2026-03-27T04:35:00Z

crates/store/src/state/sync_state.rs

+            if accumulated_size > MAX_RESPONSE_PAYLOAD_BYTES {
+                break;
+            }


Returning a single block is always guaranteed, right? We should make sure this is the case because otherwise it might be problematic

igamigo · 2026-03-27T04:35:58Z

crates/store/src/state/sync_state.rs

+
+        loop {
+            let range = current_from..=checkpoint;
+            let Some(note_sync) = self.db.get_note_sync(range, note_tags.clone()).await? else {


Wonder if get_note_sync could change to take something like &[u32] to avoid cloning every time here

igamigo · 2026-03-27T04:38:42Z

crates/store/src/state/sync_state.rs

        block_range: RangeInclusive<BlockNumber>,
-    ) -> Result<(NoteSyncUpdate, MmrProof, BlockNumber), NoteSyncError> {
+    ) -> Result<Vec<(NoteSyncUpdate, MmrProof)>, NoteSyncError> {
        let inner = self.inner.read().await;


I think we want to take the lock in the loop to avoid keeping it for the whole duration

(and it can be moved right next to the open_at() call)

igamigo · 2026-03-27T04:49:13Z

crates/store/src/state/sync_state.rs

+            if block_num >= checkpoint {
+                break;
+            }


We break here but AFAIK we are supposed to return notes for the checkpoint block, right? I think this might be a special case. If the user calls SyncMmr which returns the chain tip header and the MmrDelta, the user already has the header (and it's already validated by default). So when block_num == checkpoint, we might to just returnr the notes, and not return the merkle path at all (open_at would otherwise fail, I think). cc @bobbinth

igamigo · 2026-03-27T04:53:34Z

crates/store/src/state/sync_state.rs

+                break;
+            }
+
+            let mmr_proof = inner.blockchain.open_at(block_num, checkpoint)?;


Before, I think if the user set a block_to beyond the chain tip, the response would be automatically valid because the store would find no note after the chain tip, but now because we open_at the user-set checkpoint, this might fail. I think this is fine because it's a wrong argument, but maybe at the RPC level we should check that the block_to is not beyond the chain tip and return early.

TomasArrachea added 2 commits March 25, 2026 18:02

feat: comput emmr proof at chain tip for sync_notes

051de21

feat: return multiple blocks on SyncNotes

6e0dd69

TomasArrachea force-pushed the tomasarrachea-sync-notes-rework branch from 6961007 to 6e0dd69 Compare March 26, 2026 15:26

chore: format

cc7ce76

TomasArrachea marked this pull request as ready for review March 26, 2026 15:34

TomasArrachea requested review from Mirko-von-Leipzig and igamigo March 26, 2026 15:34

bobbinth reviewed Mar 26, 2026

View reviewed changes

proto/proto/rpc.proto Outdated Show resolved Hide resolved

TomasArrachea added 2 commits March 26, 2026 14:43

feat: update response size constants

4ddb750

review: return requested block num in the response if specified

2d16d93

TomasArrachea mentioned this pull request Mar 26, 2026

feat: move sync notes loop to the Store #1851

Merged

TomasArrachea and others added 2 commits March 26, 2026 21:21

feat: move sync notes loop to the Store (#1851)

adbefa6

Merge branch 'next' into tomasarrachea-sync-notes-rework

ff4a02b

bobbinth approved these changes Mar 27, 2026

View reviewed changes

review: fix SyncNotesResponse proto field numbering

736c937

igamigo reviewed Mar 27, 2026

View reviewed changes

		// `block_to` matches the request's `block_range.block_to`, or the chain tip if it was
		// not specified.

		@@ -130,26 +130,34 @@ impl rpc_server::Rpc for StoreApi {
		let request = request.into_inner();

		let chain_tip = self.state.latest_block_num().await;

Conversation

TomasArrachea commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Follow ups:

Uh oh!

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igamigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TomasArrachea commented Mar 25, 2026 •

edited

Loading