Skip to content

Merge getevents-v2 branch into main - #6001

Merged
urvisavla merged 3 commits into
mainfrom
getevents-v2
Sep 3, 2026
Merged

urvisavla merged 3 commits into
mainfrom
getevents-v2

Conversation

@urvisavla

Copy link
Copy Markdown
Contributor

Merge getevents-v2 branch into main

@urvisavla
urvisavla marked this pull request as ready for review September 3, 2026 21:48
Copilot AI balanced review requested due to automatic review settings September 3, 2026 21:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds getEvents v2 protocol types and validation while aligning legacy event responses.

Changes:

  • Adds v2 request, response, filtering, errors, and validation.
  • Adds comprehensive protocol tests.
  • Removes the deprecated v1 success field.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
protocols/rpc/get_events.go Removes the deprecated event field.
protocols/rpc/get_events_v2.go Defines and validates the v2 wire protocol.
protocols/rpc/get_events_v2_test.go Tests validation and JSON contracts.
CHANGELOG.md Announces getEvents v2 support.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread protocols/rpc/get_events_v2_test.go
Comment thread CHANGELOG.md
Comment thread protocols/rpc/get_events_v2.go Outdated
@karthikiyer56

Copy link
Copy Markdown
Contributor

LGTM, and approved, copilot turned up valid comments btw

Copilot AI review requested due to automatic review settings September 3, 2026 22:12
urvisavla and others added 3 commits September 3, 2026 15:12
)

* protocols/rpc: Add getEvents v2 wire types and request validation

Transcribe the getEvents v2 wire types from the accepted proposal
(https://github.com/orgs/stellar/discussions/1872): request/response/event
structs, scan statuses, error reasons and error.data payloads, and the
spec's default caps.

Valid checks every rule decidable from the request alone and returns the
first violation as a typed *InvalidParamsError carrying the proposal's
verbatim message plus the ready-to-serialize error.data payload. Rules
needing server state stay with the server: the serving window, unknown-field
rejection, json-format topic decoding, and the term budget (counting terms
requires the server's XDR-JSON converter).

The limit cap (1000) is spec-fixed and enforced as a constant; the filter
cap (256) is a provider-configurable default passed to Valid; the term
budget (15) ships as a documented constant with its error.data fields.

Tests pin the wire contract with golden JSON for requests, responses,
events, and error payloads, literal assertions for every wire constant and
message, a reflection test keeping EventInfoV2 in lockstep with v1's
EventInfo (minus the deprecated inSuccessfulContractCall), and boundary
cases for custom and zero filter caps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* protocols/rpc: address getEvents v2 review comments

- Limit becomes *uint so wire presence is preserved: nil means omitted
  (server default applies), while an explicit limit outside [1, 1000] is
  rejected with the proposal's message, closing the documented limit-0
  deviation.
- The validation test helper asserts full error messages with EqualError
  instead of Contains, actually pinning the proposal's verbatim wording.
- Document that "filters": null decodes like an omitted member (match
  all events), consistent with null topic handling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* protocols/rpc: address getEvents v2 review round 2

- Restructure Valid on the spec's union discriminator: shared checks
  (xdrFormat, limit) run first, then validCursorQuery or validRangeQuery.
  Removes the duplicated cursor guard, and cursor + invalid xdrInputFormat
  now reports mutual exclusion, the truer complaint.
- Add TopicScVal(xdr.ScValView) so callers build topic values without
  hand-assembling quoted base64 JSON; it validates the view first. The
  test helper now goes through it.
- Valid takes a defined MaxFilters type, so a v1-shaped call site
  (Valid(maxLimit)) cannot compile with the wrong cap.
- Validate topics by unmarshalling into xdr.ScValView (base64-decodes and
  structurally validates without building an ScVal tree; the Raw length
  check keeps trailing-byte strictness).
- LedgerOutOfRangeErrorData/CursorMalformedErrorData inject Reason in
  MarshalJSON so a wrong reason is unrepresentable; InvalidParamsErrorData
  documents why it is the exception.
- Reject contract IDs whose checksum-valid payload is not 32 bytes
  (strkey.Decode does not enforce length).
- Document that maxFilters must be >= 1; hasTopic via slices.ContainsFunc;
  mapKeys replaced by slices.Collect(maps.Keys).
- Tests: pin "filters": null decoding from real JSON for both query
  shapes, cover the short-payload contract ID and malformed TopicScVal.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add getEvents v2 wire types to the pending changelog

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Deprecated since protocol 23 with 'remove in v24'. The full-history RPC's
getEvents endpoint launches without it, so the removal lands ahead of the
v24 window rather than shipping a new server that still serves it.

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 4 out of 4 changed files in this pull request and generated no new comments.

@urvisavla
urvisavla merged commit d8c8acf into main Sep 3, 2026
11 checks passed
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