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.

proto/proto/rpc.proto

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.

crates/store/src/state/sync_state.rs

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)

I removed the text below; as mentioned in this discussion the block_to will always return either the requested block or the chain tip.

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

crates/store/src/server/rpc_api.rs

crates/store/src/state/sync_state.rs

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)

crates/store/src/state/sync_state.rs

bobbinth · 2026-03-28T08:58:32Z

proto/proto/rpc.proto

+        // Merkle path to verify the block's inclusion in the MMR at the returned
+        // `block_range.block_to`.
+        //
+        // An MMR proof can be constructed for the leaf of index `block_header.block_num` of
+        // an MMR of forest `block_range.block_to` with this path.
+        primitives.MerklePath mmr_path = 2;


Is the comment on line 480-481 correct here? I thought the MMR path is done against the requested block_range.block_to rather than the returned one.

bobbinth · 2026-03-28T09:18:04Z

crates/store/src/server/rpc_api.rs

+        let block_from = block_range.start();
+        // Clamp block_to to the chain tip to avoid erroring when opening the MMR proof
+        let block_to = block_range.end().min(&chain_tip);
+        let clamped_block_range = *block_from..=*block_to;


I think things like this should be enforced in BlockRange::into_inclusive_range(). But also, I'm not sure we want to clamp here. Maybe returning an error is a better approach?

Changed this to error instead 889fc04

igamigo · 2026-03-30T20:24:36Z

crates/store/src/server/rpc_api.rs

-                block_num: last_block_included.as_u32(),
+            block_range: Some(proto::rpc::BlockRange {
+                block_from: block_from.as_u32(),
+                block_to: Some(block_to.as_u32()),


block_to here is probably not what we want, as it is just the same number that the user might have requested. Instead, what we need (AFAIK) is to return the largest block num in the results (or have it returned separately on sync_notes). Or, if no valid sync notes response was found, then we reached the user's block_to

I changed this so the block_to returned marks the last block included in the response. This makes sense for the proposed flow where the client calls SyncChainMmr and then uses that chain tip as parameter for the sync notes. But if a user made a sync notes request without specifying the chain tip, it would have no way of finding out which is the chain tip used for the MMR paths included in the response. It might not be the last included block if the response is truncated.

A simple solution would be to add a chain_tip field to the response

Edit: actually, using PaginationInfo would be a better fit.

Agree, that should work well

crates/store/src/state/sync_state.rs

igamigo · 2026-03-30T20:26:42Z

crates/store/src/state/sync_state.rs

+        // Use block_end + 1 as the MMR checkpoint so that block_end itself can be proven.
+        let mmr_checkpoint = block_end + 1;


nit: I'd move this down closer to the open_at call

igamigo · 2026-03-30T20:32:54Z

proto/proto/rpc.proto

+    // If `response.block_range.block_to` is less than the requested range end, make another
+    // request starting from `response.block_range.block_to + 1` to continue syncing.


I think the underlying query is committed_at > block_from, so I think prompting for + 1 is not correct here.

bobbinth

Looks good! Thank you. I left some more comments inline - mostly about making sure we treat block ranges consistently.

crates/store/src/server/rpc_api.rs

crates/store/src/db/models/queries/notes.rs

bobbinth · 2026-03-31T10:26:05Z

crates/store/src/db/models/queries/notes.rs

 pub(crate) fn select_notes_since_block_by_tag_and_sender(
    conn: &mut SqliteConnection,
    account_ids: &[AccountId],
    note_tags: &[u32],
    block_range: RangeInclusive<BlockNumber>,
-) -> Result<(Vec<NoteSyncRecord>, BlockNumber), DatabaseError> {
+) -> Result<Vec<NoteSyncRecord>, DatabaseError> {


Not for this PR, but it seems like this function is never invoked with account_ids supplied. If so, we should remove the account_ids parameter and simplify the associated SQL. Let's create an issue for this.

crates/store/src/state/sync_state.rs

bobbinth · 2026-03-31T10:37:56Z

crates/store/src/state/sync_state.rs

+            if !results.is_empty() && accumulated_size > MAX_RESPONSE_PAYLOAD_BYTES {
+                break;
+            }


Why do we have !results.is_empty() as a condition here? Basically, if the result is empty, why do we need to fall through to getting the MMR proof?

This was added after this dicussion #1843 (comment) to guarantee that at least one update is included in the response. If the db.get_note_sync returns None we do break .

I see. Basically, the idea here is that if somehow accumulated_size is over the limit on the first iteration (i.e., when esults.is_empty()) we still want to send the response. Practically, this shouldn't happen (unless we are off in accumulated_size computations), but it's better to be on the safe side.

crates/store/src/state/sync_state.rs

SantiagoPittella

Looks good overall

crates/store/src/db/models/queries/notes.rs

bin/stress-test/src/store/mod.rs

igamigo

LGTM! Left some minor, non-blocking comments

crates/store/src/state/sync_state.rs

proto/proto/rpc.proto

igamigo · 2026-03-31T19:13:43Z

crates/store/src/state/sync_state.rs

    #[instrument(level = "debug", target = COMPONENT, skip_all, ret(level = "debug"), err)]
    pub async fn sync_notes(
        &self,
        note_tags: Vec<u32>,
        block_range: RangeInclusive<BlockNumber>,
-    ) -> Result<(NoteSyncUpdate, MmrProof, BlockNumber), NoteSyncError> {
-        let inner = self.inner.read().await;
-
-        let (note_sync, last_included_block) =
-            self.db.get_note_sync(block_range, note_tags).await?;
+    ) -> Result<(Vec<(NoteSyncUpdate, MmrProof)>, BlockNumber), NoteSyncError> {


With some of these changes we might need to update the store's README.md

igamigo · 2026-03-31T19:18:57Z

crates/store/src/db/models/queries/notes.rs

 ///     -- filter the block's notes and return only the ones matching the requested tags or senders
 ///     (tag IN (?1) OR sender IN (?2))
 /// ```
 pub(crate) fn select_notes_since_block_by_tag_and_sender(


Could this be modified to return multiple responses at once? I think this was the original suggestion (although we should do it in a separate issue/PR)

igamigo · 2026-03-31T19:41:08Z

crates/store/src/state/sync_state.rs

+        // if results is empty, return `block_end` since the sync is complete.
+        let last_block_checked =
+            results.last().map_or(block_end, |(update, _)| update.block_header.block_num());


There is a small optimization here where we return the block number we dropped, minus one. For example, if we match against blocks 1, 5, 10 and 15 but block 15 does not make it into the response, instead of returning 10 as the last included block, we return 14 (so the next query avoids blocks 10-14 because the user will start at 15 already). I doubt this is significant due to how the tables are probably indexed, so I would not bother doing it unless it's really trivial, but maybe it can be done in a follow up PR or issue

Created #1870

bobbinth

All looks good! Thank you!

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

TomasArrachea and others added 6 commits March 27, 2026 11:03

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

01823ca

review: remove comment on proto SyncNotesRespose

cbb997a

review: address small comments

12401ce

review: set block_to to chain tip if requested number is beyond the tip

e8fea10

review: return last note update without mmr proof

727099c

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

06a85a9

bobbinth reviewed Mar 28, 2026

View reviewed changes

bobbinth and others added 6 commits March 29, 2026 16:05

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

b9532b2

review: fix mmr proof comment

0e24e08

review: error if block_to is greater than chain tip

889fc04

review: use block_end + 1 as mmr checkpoint

59c4658

chore: update proto response comment

0830662

chore: changelog

46495d2

test: mmr proof

31cdafd

TomasArrachea requested a review from igamigo March 30, 2026 17:26

igamigo reviewed Mar 30, 2026

View reviewed changes

TomasArrachea added 7 commits March 30, 2026 18:11

review: return the last checked block

d6e473c

chore: lint

edb9a6c

chore: format

86505ae

feat: update proto comment

168ae82

feat: include chain tip in the response

41d1036

feat: use block_num + 1 for sync pagination

21607ad

feat: update proto comment

b5debf9

bobbinth reviewed Mar 31, 2026

View reviewed changes

TomasArrachea added 2 commits March 31, 2026 11:01

review: update select notes sql comment

49854da

review: add comment on MMR proof

0601592

SantiagoPittella reviewed Mar 31, 2026

View reviewed changes

crates/store/src/db/models/queries/notes.rs Show resolved Hide resolved

bin/stress-test/src/store/mod.rs Show resolved Hide resolved

review: fix break condition on the stress test

29fb8a2

This was referenced Mar 31, 2026

Validate block_to is not greater than chain tip #1866

Open

Add serialized size to structs used in gRPC responses #1867

Open

SantiagoPittella approved these changes Mar 31, 2026

View reviewed changes

TomasArrachea added 2 commits March 31, 2026 15:34

chore: typo

6a2088e

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

cead199

igamigo approved these changes Mar 31, 2026

View reviewed changes

igamigo mentioned this pull request Mar 31, 2026

NTX builder may be loading duplicate IDs #1868

Closed

review: update store README

ce575ae

bobbinth approved these changes Mar 31, 2026

View reviewed changes

bobbinth merged commit a7abf73 into next Mar 31, 2026
18 checks passed

bobbinth deleted the tomasarrachea-sync-notes-rework branch March 31, 2026 20:29

This was referenced Mar 31, 2026

Optimize block num returned on SyncNotes #1870

Open

Move SyncNotes loop to the db #1871

Open

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

		// Use block_end + 1 as the MMR checkpoint so that block_end itself can be proven.
		let mmr_checkpoint = block_end + 1;

		// If `response.block_range.block_to` is less than the requested range end, make another
		// request starting from `response.block_range.block_to + 1` to continue syncing.

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SantiagoPittella left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

igamigo left a comment

TomasArrachea commented Mar 25, 2026 •

edited

Loading

TomasArrachea Mar 30, 2026 •

edited

Loading

TomasArrachea Mar 30, 2026 •

edited

Loading

TomasArrachea Mar 31, 2026 •

edited

Loading

SantiagoPittella left a comment •

edited

Loading