2518 - Generic Pagination Engine with Cursor Pagination Policy + Importer Cursor Pagination

[gventino-cw]

PR Type

Enhancement

---

Description

• Introduce generic cursor-based pagination engine

• Delegate block fetches to ImporterPaginationClient

• Add server-side pagination in RPC handlers

• Extend ExternalBlock for merging paginated transactions

---

Diagram Walkthrough

flowchart LR
  BC["BlockchainClient.fetch_block_and_receipts"]
  IPC["ImporterPaginationClient.new"]
  PPF["PaginatedPageFetcher.collect"]
  HTTP["http.request"]
  RED["PageReducer.reduce"]
  OUT["ExternalBlockWithReceipts"]
  BC -- "calls" --> IPC
  IPC -- "uses" --> PPF
  PPF -- "fetches pages" --> HTTP
  HTTP -- "returns page" --> PPF
  PPF -- "applies reducer" --> RED
  RED -- "finish" --> OUT

Loading

File Walkthrough

	Relevant files
Formatting	1 files importer_supervisor.rs Wrap FakeLeader arm in braces for consistency                        +3/-2
Enhancement	6 files blockchain_client.rs Delegate fetching to ImporterPaginationClient                        +4/-23    importer_pagination.rs Add ImporterPaginationClient pagination logic                        +249/-0  server.rs Implement pagination in RPC handlers                                          +27/-2    importer_pagination.rs Define ImporterPagination request/response types                  +200/-0  pagination.rs Introduce generic pagination engine and policies                  +220/-0  external_block.rs Add methods for transaction merging and count                        +32/-0
Configuration changes	2 files mod.rs Expose importer_pagination in module                                          +1/-0      mod.rs Re-export pagination types in types module                              +11/-0

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 2518 - Fully compliant Compliant requirements: Implement response pagination to avoid importer stopping when RPC responses exceed max_response_size_bytes. Provide a mechanism to split or stream large RPC responses. Add pagination support in RPC handlers for stratus_getBlockAndReceipts and stratus_getBlockWithChanges. Extend the importer to merge paginated block data (transactions, receipts, changes).

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Invalid format strings Multiple bail! invocations use brace-style interpolation ({context}, {transactions_len}, {expected_total}) without supplying matching format arguments, leading to compilation failures. Use positional {} placeholders with corresponding arguments or named parameters accepted by format!. fn validate_progress(page: &ImporterPageInfo, expected_total: &mut Option<usize>, context: &str) -> anyhow::Result<Option<String>> { if page.returned == 0 && page.next_cursor.is_some() { bail!("paginated {context} returned no items but provided a next cursor"); } match expected_total { Some(expected_total) if *expected_total != page.total => { bail!("paginated {context} changed total from {expected_total} to {}", page.total); } Some(_) => {} None => *expected_total = Some(page.total), } Ok(page.next_cursor.clone()) } struct BlockAndReceiptsPages { block_number: BlockNumber, block: Option<ExternalBlock>, receipts: Vec<ExternalReceipt>, expected_total: Option<usize>, } impl BlockAndReceiptsPages { fn new(block_number: BlockNumber) -> Self { Self { block_number, block: None, receipts: Vec::new(), expected_total: None, } } fn push_block(&mut self, page_block: ExternalBlock) -> anyhow::Result<()> { if page_block.number() != self.block_number { bail!( "paginated block with receipts returned unexpected block number {} instead of {}", page_block.number(), self.block_number ); } match &mut self.block { Some(block) => block.extend_full_transactions_from(page_block), None => { self.block = Some(page_block); Ok(()) } } } } impl PageReducer<BlockAndReceiptsPageResponse> for BlockAndReceiptsPages { type Output = ExternalBlockWithReceipts; type Paginator = ImporterCursorPaginator; fn reduce(&mut self, page: BlockAndReceiptsPageResponse) -> anyhow::Result<Option<String>> { let cursor = validate_progress(&page.pagination, &mut self.expected_total, "block with receipts")?; let page_block = ExternalBlock::try_from(page.block)?; self.push_block(page_block)?; self.receipts.extend(page.receipts); Ok(cursor) } fn finish_after_not_found(self) -> anyhow::Result<Option<Self::Output>> { if self.block.is_none() { Ok(None) } else { bail!("block disappeared while fetching paginated block with receipts"); } } fn finish(self) -> anyhow::Result<Option<Self::Output>> { let Some(block) = self.block else { return Ok(None); }; let expected_total = self.expected_total.unwrap_or_default(); let transactions_len = block.full_transactions_len()?; if transactions_len != expected_total { bail!("paginated block with receipts assembled {transactions_len} transactions but expected {expected_total}"); }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix bail! formatting placeholders Replace the named placeholders in the bail! macros with positional {} placeholders and pass the corresponding variables as arguments. This ensures the format strings compile correctly and include the intended context and values. src/eth/rpc/blockchain_client/importer_pagination.rs [66-80] fn validate_progress(page: &ImporterPageInfo, expected_total: &mut Option<usize>, context: &str) -> anyhow::Result<Option<String>> { if page.returned == 0 && page.next_cursor.is_some() { - bail!("paginated {context} returned no items but provided a next cursor"); + bail!( + "paginated {} returned no items but provided a next cursor", + context + ); } match expected_total { - Some(expected_total) if *expected_total != page.total => { - bail!("paginated {context} changed total from {expected_total} to {}", page.total); + Some(total) if *total != page.total => { + bail!( + "paginated {} changed total from {} to {}", + context, + total, + page.total + ); } Some(_) => {} None => *expected_total = Some(page.total), } Ok(page.next_cursor.clone()) } Suggestion importance[1-10]: 9 __ Why: This fixes compile errors in validate_progress by replacing unsupported named placeholders with positional {} formatting, enabling the bail! macros to work correctly.	High
	Use positional placeholders in bail! Correct the format string in the bail!macro by using {} placeholders for both values and passing page_block_number and self.block_number as positional arguments to avoid compile errors. src/eth/rpc/blockchain_client/importer_pagination.rs [178-184] impl BlockWithChangesPages { fn push_block(&mut self, page_block: BlockRocksdb) -> anyhow::Result<()> { let page_block_number = BlockNumber::from(page_block.header.number); if page_block_number != self.block_number { bail!( - "paginated block with changes returned unexpected block number {page_block_number} instead of {}", + "paginated block with changes returned unexpected block number {} instead of {}", + page_block_number, self.block_number ); } // ... } } Suggestion importance[1-10]: 8 __ Why: Changing the bail! macro to use {} placeholders and positional arguments corrects the format string error in push_block, ensuring it compiles and displays the values properly.	Medium
	Correct bail! placeholders in finish Replace the named placeholders in the bail! call with positional {} and supply total and expected_total as arguments so the error message formats correctly. src/eth/rpc/blockchain_client/importer_pagination.rs [236-244] impl PageReducer<BlockWithChangesPageResponse> for BlockWithChangesPages { fn finish(self) -> anyhow::Result<Option<Self::Output>> { let Some(block) = self.block else { return Ok(None); }; let expected_total = self.expected_total.unwrap_or_default(); let total = block.transactions.len() + self.changes.account_changes.len() + self.changes.slot_changes.len(); if total != expected_total { - bail!("paginated block with changes assembled {total} items but expected {expected_total}"); + bail!( + "paginated block with changes assembled {} items but expected {}", + total, + expected_total + ); } Ok(Some((block, self.changes))) } } Suggestion importance[1-10]: 8 __ Why: Updating the bail! call in finish to use positional formatting fixes a compilation issue and ensures the error message includes the correct total and expected_total values.	Medium

[cloudwalk-review-agent]

Summary

Great change set. The pagination design is cohesive across server slicing (ImporterPagination), client reassembly (PaginatedPageFetcher + reducers), and domain validation (ExternalBlock transaction merge helpers). I reviewed for correctness/safety around partial data assembly, cursor integrity, and mismatch detection; the implementation has strong defensive checks (stable total, block identity/hash checks, duplicate state-change guards, and not-found-after-partial failure behavior).

Test coverage is substantial and targeted at failure modes that matter for importer safety (cursor parsing, bounds handling, multi-section slicing, mismatch/error paths, and reducer finalization). I did not find any concrete blocking issues in the provided diff/context.

[cloudwalk-review-agent]

Summary

Solid implementation of cursor pagination across server and importer sides, with strong defensive checks for consistency (stable totals, block identity/hash continuity, duplicate change detection, and partial-not-found failure). The generic paginator/fetcher abstractions are coherent, and test coverage is meaningful across happy paths and important failure modes.

I did not find any concrete blocking issues in the provided diff/context.

[carneiro-cw]

Did not fully review yet, but add an e2e test for: requests that need pagination, requests that dont but ask for pagination, legacy requests without the pagination param.

edit: i guess the last one already exists

[gventino-cw]

Did not fully review yet, but add an e2e test for: requests that need pagination, requests that dont but ask for pagination, legacy requests without the pagination param.

edit: i guess the last one already exists

ok, will do it

[carneiro-cw]

Also would be good to understand the behavior if say, a follower is deployed with this new feature, but leader does not have it yet. Is that follower prepared to deal with that?
This is important for when we first deploy this. Either make the follower deal with either response version or add a config to enable/disable pagination.

[cloudwalk-review-agent]

Summary

Nice progress on introducing generic cursor pagination and importer-side reassembly; the reducer validation and tests are generally strong. I found one blocking deployment-safety issue related to mixed-version leader/follower compatibility.

Blocking

• The importer client now always calls stratus_getBlockAndReceipts / stratus_getBlockWithChanges with a second pagination param and expects paginated response shapes. If follower is upgraded before leader (raised in PR discussion), older leaders that only support legacy signatures/tuple responses can fail decode or reject params, causing importer sync interruption during rollout.

Please add backward compatibility in the client fetch path (e.g., fallback to legacy one-param call and legacy response decode when paginated call fails with method/params/shape mismatch), or gate pagination usage behind a config/feature flag for staged deploys.

Other than that, coverage and invariants look good.

[gventino-cw]

Also would be good to understand the behavior if say, a follower is deployed with this new feature, but leader does not have it yet. Is that follower prepared to deal with that? This is important for when we first deploy this. Either make the follower deal with either response version or add a config to enable/disable pagination.

currently old follower -> new leader works fine. But old leader <- new follower it will not work, because the follower at this version always send the request expecting a paginated response.

[cloudwalk-review-agent]

Summary

Solid implementation overall: pagination reducers, cursor decoding, reassembly invariants, and test coverage are strong. However, there is one blocking deploy-safety issue still present in the importer client path.

Blocking

The follower importer now always sends the paginated 2-param RPC form and expects paginated/object-compatible responses, but there is no request-level fallback when talking to an older leader that only supports legacy signatures/response shapes. In a mixed-version rollout (new follower -> old leader), this can fail RPC decode/params validation and interrupt syncing.

[gventino-cw]

Also would be good to understand the behavior if say, a follower is deployed with this new feature, but leader does not have it yet. Is that follower prepared to deal with that? This is important for when we first deploy this. Either make the follower deal with either response version or add a config to enable/disable pagination.

currently old follower -> new leader works fine. But old leader <- new follower it will not work, because the follower at this version always send the request expecting a paginated response.

fixed it

[stratus-benchmark]

Benchmark:
Run ID: bench-3e4f02af

Git Info:

• Branch: feat/2518-pagination
• Commit: d3085fd
• PR: 2518 - Generic Pagination Engine with Cursor Pagination Policy + Importer Cursor Pagination #2626

Leader Stats:
RPS Stats: Max: 8394.00, Min: 1978.00, Avg: 3045.58, StdDev: 438.74
TPS Stats: Max: 3351.00, Min: 17.00, Avg: 2975.75, StdDev: 334.69

Follower Stats:
Imported Blocks/s: Max: 5.00, Min: 1.00, Avg: 2.06, StdDev: 0.86
Imported Transactions/s: Max: 11575.00, Min: 2744.00, Avg: 6132.46, StdDev: 2519.63

Plots:

• View Leader Plot
• View Follower Plot

[cloudwalk-review-agent]

Summary

Great work — this PR introduces a cohesive pagination model across server and importer client with strong compatibility handling and meaningful guardrails.

What I verified in the provided diff/context:

• Server now supports optional cursor pagination for both stratus_getBlockAndReceipts and stratus_getBlockWithChanges, while preserving legacy one-shot shapes when pagination is not needed.
• Importer client fetch path now supports both paginated and legacy response forms (BlockWithChangesPageResponse custom deserializer + optional pagination), addressing mixed-version rollout risk.
• Reducers include solid consistency checks (stable total, cursor monotonicity, block identity/hash continuity, duplicate change detection, tx/receipt count matching, partial-not-found fail-fast).
• Test coverage is substantial and includes e2e round-trips plus targeted unit tests for cursor decoding, clamping, section slicing, byte-budget behavior, backward compatibility, and reducer invariants.

I did not find any concrete blocking issues in the shown changes.

[stratus-benchmark]

Benchmark:
Run ID: bench-2b42bcfa

Git Info:

• Branch: feat/2518-pagination
• Commit: 15962ab
• PR: 2518 - Generic Pagination Engine with Cursor Pagination Policy + Importer Cursor Pagination #2626

Leader Stats:
RPS Stats: Max: 10084.00, Min: 2559.00, Avg: 3051.53, StdDev: 425.93
TPS Stats: Max: 3380.00, Min: 450.00, Avg: 2989.00, StdDev: 292.72

Follower Stats:
Imported Blocks/s: Max: 7.00, Min: 1.00, Avg: 4.61, StdDev: 1.01
Imported Transactions/s: Max: 17864.00, Min: 3325.00, Avg: 13767.50, StdDev: 2895.16

Plots:

• View Leader Plot
• View Follower Plot

[stratus-benchmark]

Benchmark:
Run ID: bench-436fffc2

Git Info:

• Branch: feat/2518-pagination
• Commit: a9cd119
• PR: 2518 - Generic Pagination Engine with Cursor Pagination Policy + Importer Cursor Pagination #2626

Leader Stats:
RPS Stats: Max: 8433.00, Min: 1525.00, Avg: 2970.45, StdDev: 374.99
TPS Stats: Max: 3424.00, Min: 264.00, Avg: 2928.45, StdDev: 320.40

Follower Stats:
Imported Blocks/s: Max: 8.00, Min: 2.00, Avg: 4.75, StdDev: 1.08
Imported Transactions/s: Max: 23744.00, Min: 873.00, Avg: 13910.16, StdDev: 3443.14

Plots:

• View Leader Plot
• View Follower Plot