Skip to content

chore(store): generalize pagination for the store - #2737

Open
SantiagoPittella wants to merge 8 commits into
nextfrom
santiagopittella-store-pagination-trait
Open

SantiagoPittella wants to merge 8 commits into
nextfrom
santiagopittella-store-pagination-trait

Conversation

@SantiagoPittella

Copy link
Copy Markdown
Collaborator

closes #2730

Adds a Paginated trait to the store and moves all paginated reads onto it. Queries return a shared Page { items, next}. Db::page reads one page and Db::pages streams all of them, replacing hand-written loops. Each query had its own page type and progress reporting. One contract keeps them consistent and makes paginating inside a stream trivial for future streaming endpoints.

Also:

  • Vault and storage map sync now return OUT_OF_RANGE when one block exceeds the page limit, instead of an empty page that skipped it.
  • block_num is the last fully covered block, so it can be further ahead than the last row. SyncNotes returns block_to when done. proto is unchanged.
  • Each page runs in its own read transaction.

Changelog

[[entry]]
scope       = "rpc"
impact      = "fixed"
description = "`SyncAccountVault` and `SyncAccountStorageMaps` return `OUT_OF_RANGE` when one block exceeds the page limit, instead of skipping it."

[[entry]]
scope       = "rpc"
impact      = "changed"
description = "`SyncNotes` reports `block_to` in `pagination_info.block_num` when no matching notes remain."

@SantiagoPittella

Copy link
Copy Markdown
Collaborator Author

The Paginated trait has one method: page(after), which returns the page that comes after a given cursor (or the first page if there's no cursor). It returns a Page, a small struct with two fields: items (the results), and next (the cursor to pass in to get the following page), or None when there's nothing left. db.page(..) reads one page, and db.pages(..) reads all pages in order as a stream.

@SantiagoPittella SantiagoPittella changed the title chore: generiliza pagination for the store chore: generalize pagination for the store Oct 5, 2026
@SantiagoPittella SantiagoPittella changed the title chore: generalize pagination for the store chore(store): generalize pagination for the store Oct 5, 2026
Comment on lines +31 to +41
after: Option<&AccountId>,
) -> Result<Page<(AccountId, Word), AccountId>, DatabaseError> {
// Fetch one extra to determine if there are more results
let limit = i64::try_from(self.page_size.get() + 1).expect("page size fits within i64");

let commitments = match after {
Some(cursor) => {
tx.query(SQL_AFTER_CURSOR, &[&limit, &VALID_FOREVER, cursor], commitment_from_row)?
},
None => tx.query(SQL_FIRST_PAGE, &[&limit, &VALID_FOREVER], commitment_from_row)?,
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if perhaps we shouldn't use next: &AccountId instead of after: Option<T>? In general for all the pagination; this would remove a layer of mapping/indirection I think.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might then also make sense to have Cursor::default so that can do .map(..).unwrap_or_default().

Though that might not fit the current shape very well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed to something similar. For the account ids and nullifier queries the cursor is Option<AccountId/Nullifier> and None means to start from the first row. An empty blob sorts before every key, so I reintroduced the account_id >= ? statements that we were using before, and the match and the second SQL file are gone (we were using one SQL file for first page, another for the others).

@kkovaacs kkovaacs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, thanks!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pagination standardization

3 participants