Skip to content

feat(api): add pagination to validators list#2703

Open
julienh-ssv wants to merge 5 commits intostagefrom
feat/pagination-validators-list
Open

feat(api): add pagination to validators list#2703
julienh-ssv wants to merge 5 commits intostagefrom
feat/pagination-validators-list

Conversation

@julienh-ssv
Copy link
Contributor

@julienh-ssv julienh-ssv commented Feb 26, 2026

Add optional pagination to the validators http api with retro-compatibility

curl --request GET \
    --url 'http://ssv-node-exporter.stage.vnet.ops.ssvlabsinternal.com/v1/validators' \
    --header 'Content-Type: application/json' \
    --data '{
      "clusters": [
        [73,74,75,76]
      ],
      "pagination": {
        "page": 1,
        "per_page": 10
      }
    }'

@julienh-ssv julienh-ssv self-assigned this Feb 26, 2026
@julienh-ssv julienh-ssv requested review from a team as code owners February 26, 2026 12:55
@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

❌ Patch coverage is 87.87879% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.2%. Comparing base (a555c14) to head (79bec9b).
⚠️ Report is 1 commits behind head on stage.

Files with missing lines Patch % Lines
api/pagination.go 84.3% 7 Missing and 3 partials ⚠️
api/handlers/validators/validators.go 93.7% 1 Missing and 1 partial ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR adds optional pagination to the validators HTTP API endpoint with full backwards compatibility. When no page or per_page query parameters are provided, the response shape is unchanged (just data). When pagination is requested, the response includes a pagination object with page, per_page, total, and total_pages, and results are sorted deterministically by validator public key.

  • A new reusable api/pagination.go module provides OptionalPaginationFromQuery (parsing + validation), SliceBounds (overflow-safe slice indexing), and PaginationFromRequest (response metadata).
  • The validators handler in validators.go branches between paginated and non-paginated paths to maintain retro-compatibility.
  • The mock in tests was updated to use slices.Clone to match real sharesStorage.List behavior (returns a new slice), which is necessary because sort.Slice is now called on the returned slice.
  • SliceBounds and PaginationFromRequest lack dedicated unit tests despite containing non-trivial arithmetic.

Confidence Score: 4/5

  • This PR is safe to merge — the backwards-compatible design ensures no breaking changes for existing API consumers.
  • Score of 4 reflects a well-designed feature with clean separation of concerns, proper backwards compatibility, and good test coverage. Deducted 1 point for the missing unit tests on SliceBounds and PaginationFromRequest, and a minor style concern with the ceiling division pattern in PaginationFromRequest.
  • api/pagination.go — the TotalPages ceiling division could overflow for extreme uint64 values; api/pagination_test.go — missing test coverage for SliceBounds and PaginationFromRequest.

Important Files Changed

Filename Overview
api/pagination.go New generic pagination library with OptionalPaginationFromQuery, SliceBounds, and PaginationFromRequest. Well-designed with overflow-safe slice bounds, though ceiling division in TotalPages uses a pattern that could overflow for extreme uint64 values.
api/pagination_test.go Unit tests for OptionalPaginationFromQuery cover key validation scenarios, but SliceBounds and PaginationFromRequest lack dedicated tests despite containing non-trivial arithmetic.
api/handlers/validators/validators.go Adds optional pagination to the validators list handler with full backwards compatibility. Non-paginated requests return the original response shape; paginated requests add sorting and a pagination field. Clean implementation.
api/handlers/validators/validators_test.go Thorough pagination integration tests covering backwards compatibility, page/per_page combinations, defaults, sort ordering, and validation. Mock updated with slices.Clone to match real storage behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["GET /validators"] --> B{"page or per_page\nin query?"}
    B -- No --> C["List shares with filters"]
    C --> D["Return {data: [...]}"]
    B -- Yes --> E["Parse & validate\npagination params"]
    E -- Invalid --> F["Return 400 Bad Request"]
    E -- Valid --> G["List shares with filters"]
    G --> H["Sort by ValidatorPubKey"]
    H --> I["Compute SliceBounds\n(start, end)"]
    I --> J["Slice shares[start:end]"]
    J --> K["Return {data: [...],\npagination: {...}}"]
Loading

Last reviewed commit: 54fe4e3

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@julienh-ssv julienh-ssv force-pushed the feat/pagination-validators-list branch from e6e54ea to 9917396 Compare February 26, 2026 13:25
iurii-ssv
iurii-ssv previously approved these changes Feb 27, 2026
Copy link
Contributor

@iurii-ssv iurii-ssv left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@oleg-ssvlabs oleg-ssvlabs left a comment

Choose a reason for hiding this comment

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

gj!

I think we also need to update Open API spec

@julienh-ssv julienh-ssv requested a review from oleg-ssvlabs March 2, 2026 11:54
oleg-ssvlabs
oleg-ssvlabs previously approved these changes Mar 2, 2026
Copy link
Contributor

@oleg-ssvlabs oleg-ssvlabs left a comment

Choose a reason for hiding this comment

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

lgtm

@y0sher
Copy link
Contributor

y0sher commented Mar 3, 2026

@julienh-ssv This is a breaking change in the API, can we still use v1/validators without pagination the same way?

@julienh-ssv
Copy link
Contributor Author

@julienh-ssv This is a breaking change in the API, can we still use v1/validators without pagination the same way?

@y0sher yes it's fully retro-compatible

Copy link
Contributor

@iurii-ssv iurii-ssv left a comment

Choose a reason for hiding this comment

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

Looks good, got just 1 question about an edge-case

oleg-ssvlabs
oleg-ssvlabs previously approved these changes Mar 3, 2026
@julienh-ssv julienh-ssv requested a review from oleg-ssvlabs March 3, 2026 10:59
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.

4 participants