-
Notifications
You must be signed in to change notification settings - Fork 3
Support validators by node-id/entity-id #1222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
456791b to
711d298
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for filtering validators by entity ID or node ID in the /v1/consensus/validators endpoint. Users can now query validators using an id parameter that accepts base64-encoded ed25519 public keys corresponding to either the entity ID or node ID.
Key Changes
- Added
idquery parameter to the validators endpoint that filters by entity or node ID - Updated database query to support filtering by both entity and node IDs
- Added comprehensive E2E test coverage for the new filtering capability
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| api/spec/v1.yaml | Added id query parameter documentation for validator filtering |
| storage/client/client.go | Passed new id parameter to database query |
| storage/client/queries/queries.go | Updated SQL query to filter by entity or node ID |
| tests/e2e_regression/run.sh | Fixed shell script compatibility for macOS and Linux |
| tests/e2e_regression/*/test_cases.sh | Added test cases for entity ID and node ID filtering |
| tests/e2e_regression//expected/ | Added expected responses for new test cases |
| .changelog/1222.feature.md | Added changelog entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/e2e_regression/run.sh
Outdated
| # The command to invoke psql (complete with connection params) | ||
| # HACK: Assuming `make` returns a docker command, sed removes -i and -t flags because we'll be running without a TTY. | ||
| psql="$(make --dry-run --no-print-directory psql | sed -E 's/\s-i?ti?\s/ /')" | ||
| psql="$(make --dry-run --no-print-directory psql | sed -E 's/ -i?t?i?t? / /g')" |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern -i?t?i?t? can match unexpected flag combinations. The intent appears to be removing -i, -t, -it, or -ti flags, but this pattern could match invalid sequences like -itt or -tit. Use the pattern s/ -(it|ti|i|t) / /g instead to explicitly match only valid flag combinations.
| psql="$(make --dry-run --no-print-directory psql | sed -E 's/ -i?t?i?t? / /g')" | |
| psql="$(make --dry-run --no-print-directory psql | sed -E 's/ -(it|ti|i|t) / /g')" |
d9ceb42 to
8667faf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8667faf to
93515a1
Compare
93515a1 to
0a3a6a3
Compare
Fixes: #1218
TODO: