feat: add pagination to CheckNullifiersByPrefix by SantiagoPittella · Pull Request #1191 · 0xMiden/node

SantiagoPittella · 2025-09-01T21:00:18Z

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

bobbinth

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

bobbinth · 2025-09-03T00:44:12Z

docs/src/user/rpc.md

@@ -30,9 +30,15 @@ Request proofs for a set of nullifiers.

 ## CheckNullifiersByPrefix


I'm thinking we should rename this to SyncNullifiers endpoint. This would be consistent with having all endpoints which may return "chunked" data to have a Sync prefix.

I'd prefer to have a Paged-prefix or similar, Sync implies what you do with the data, but imho it's more useful to describe the behavior of the function at hand.

Should I opened an issue to discuss whether to use Sync, Paged or another prefix?

I'd still probably prefer with SyncNullifiers. One reason is that I'd prefer our endpoints to start with an action verb (though, we could of course do GetPagedNullifiers). The other reason I think it better describes what this endpoint is for - i.e., it is for synchronizing the state of the client with the state of the network (and it should be used together with other Sync* endpoints).

I would do the renaming in this PR.

Just pushed a commit renaming to SyncNullifiers.

proto/proto/store/rpc.proto

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

Mirko-von-Leipzig · 2025-09-03T07:26:34Z

crates/store/src/state.rs

+        block_from: BlockNumber,
+        block_to: BlockNumber,


Could we use ranges here to make the inclusive/exclusive explicit?

I can either do this in this PR or as part of #1204 , wdyt?

Mirko-von-Leipzig · 2025-09-03T07:27:42Z

crates/store/README.md


-Returns a list of nullifiers that match the specified prefixes and are recorded in the node.
+Returns nullifier synchronization data for a set of prefixes within a given block range. This method allows
+clients to efficiently track nullifier creation by retrieving only the nullifiers produced between two blocks.


Suggested change

clients to efficiently track nullifier creation by retrieving only the nullifiers produced between two blocks.

clients to efficiently track nullifier creation by retrieving only the nullifiers produced within a specific range of blocks.

Mirko-von-Leipzig · 2025-09-03T07:29:51Z

crates/store/src/server/rpc_api.rs

        }

-        let nullifiers = self
+        let chain_tip = self.state.latest_block_num().await.as_u32();


Nit: I think probably makes more sense to add .as_u32() in the response segment.

Suggested change

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

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

proto/proto/store/rpc.proto

crates/store/src/db/mod.rs

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

Mirko-von-Leipzig · 2025-09-03T07:39:47Z

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

+    pub const MAX_PAYLOAD_BYTES: usize = 2 * 1024 * 1024; // 2 MB
+    pub const NULLIFIER_BYTES: usize = 32; // digest size
+    pub const ROW_OVERHEAD_BYTES: usize = NULLIFIER_BYTES + size_of::<i64>(); // 40 bytes
+    pub const ROW_LIMIT: usize = (MAX_PAYLOAD_BYTES / ROW_OVERHEAD_BYTES) + 1;


I know we've done this previously, but I would prefer calling this MAX_ROWS and then

// Request an additional row so we can determine whether this is the last page. .limit(i64::try_from(MAX_ROWS + 1).expect("limit fits in i64"))

drahnr · 2025-09-03T10:15:25Z

crates/store/src/state.rs

+    ) -> Result<(Vec<NullifierInfo>, BlockNumber), DatabaseError> {
        self.db
-            .select_nullifiers_by_prefix(prefix_len, nullifier_prefixes, block_num)
+            .select_nullifiers_by_prefix(prefix_len, nullifier_prefixes, block_from, block_to)


Out of scope: I propose introducing a Range like type to simplify our API and reduce the need to document inclusive/exclusive boundaries.

drahnr

Bar the open comments, looks good!

@SantiagoPittella @Mirko-von-Leipzig I'd defer the BlockRange proto / Range rust type refactor to a separate follow up PR / #1193 if this lands before that.

bobbinth

Looks good! Thank you!

feat: add pagination to CheckNullifiersByPrefix

20c090b

SantiagoPittella force-pushed the santiagopittella-check-nullifiers-pagination branch from 84ed02b to 20c090b Compare September 1, 2025 21:00

SantiagoPittella requested review from Mirko-von-Leipzig, bobbinth, drahnr and igamigo and removed request for bobbinth September 2, 2025 13:22

drahnr reviewed Sep 2, 2025

View reviewed changes

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

drahnr reviewed Sep 2, 2025

View reviewed changes

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

SantiagoPittella added 2 commits September 2, 2025 11:58

review:expand docs on block range inclusion

79a8585

review: use if let removing unwrap

1ac5226

SantiagoPittella requested a review from drahnr September 2, 2025 17:43

Merge branch 'next' into santiagopittella-check-nullifiers-pagination

3d029dd

bobbinth reviewed Sep 3, 2025

View reviewed changes

Mirko-von-Leipzig reviewed Sep 3, 2025

View reviewed changes

drahnr reviewed Sep 3, 2025

View reviewed changes

drahnr approved these changes Sep 3, 2025

View reviewed changes

This was referenced Sep 3, 2025

Order Sync* Requests fields #1203

Closed

Implement BlockRange message #1204

Closed

SantiagoPittella added 8 commits September 3, 2025 14:25

review: reorder paginated responses fields

810adbd

review: increase limit to 2.5MB

0e471e6

review: improve wording in readme

f62c83c

review: move BlockNumber casting to response creation

22f5ce6

review: rename to MAX_ROWS

624e833

review: rename to MAX_ROWS in pre-existing queries

477464e

Merge branch 'next' into santiagopittella-check-nullifiers-pagination

cf42040

review: rename CheckNullifiersByPrefix to SyncNullifiers

7ff3535

bobbinth approved these changes Sep 3, 2025

View reviewed changes

SantiagoPittella requested a review from Mirko-von-Leipzig September 3, 2025 19:25

SantiagoPittella merged commit 054fed7 into next Sep 3, 2025
6 checks passed

SantiagoPittella deleted the santiagopittella-check-nullifiers-pagination branch September 3, 2025 19:57

		@@ -30,9 +30,15 @@ Request proofs for a set of nullifiers.

		## CheckNullifiersByPrefix

	clients to efficiently track nullifier creation by retrieving only the nullifiers produced between two blocks.
	clients to efficiently track nullifier creation by retrieving only the nullifiers produced within a specific range of blocks.

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

Conversation

SantiagoPittella commented Sep 1, 2025

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

drahnr 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!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

drahnr left a comment •

edited

Loading