RPCv1 XDR views: migrate getEvents

[cjonas9]

What

Moves RPCv1 getEvents onto XDR views. The filter loop and the response renderer both paid for Unmarshals of every scanned xdr.DiagnosticEvent; now nothing in the query path decodes an event.

Handler-local matcher (methods/event_filter.go): a view port of the SDK's GetEventsRequest.Matches. Filters compile once per request into wire-form comparison values (decoded contract ids, marshaled topic segments, a trailing ** as an arity flag) and match against the raw bytes the view hands back. rpcv2's stores/event and its v1Filters shim are untouched.

RPCv1 query path:

• store.ScanFunction -> store.ViewScanFunction: takes xdr.DiagnosticEventView, returns (bool, error).
• sqlitedb.GetEvents reads rows as sql.RawBytes and wraps them as views; scanner errors abort the scan.
• methods/get_events.go locates each event's fields once via Fields(), matches, and renders matches inside the scan callback. Topics are read only after type and contract id pass. The callback's own error is captured and returned unchanged, as getTransactions does, so a render failure keeps its system error code.

Shared renderer: methods.EventInfoFromView replaces both eventInfoForEvent and the rpcv2 eventInfoV2 body. ev.Fields() and v0.Fields() locate type, contract id, topics and data in one pass each and return trimmed views, so their bytes are used directly with no Raw() re-walks. The type name comes from a switch instead of the SDK's per-call map. rpcv2 wraps it and rejects non contract/system types on the returned name. Wire output is byte-identical.

Why

See epic #732.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Migrates RPCv1 event filtering and response generation to zero-copy XDR views while sharing matching logic with RPCv2.

Changes:

• Adds shared view-based event filter compilation and matching.
• Updates SQLite scanning and RPC response construction to use XDR views.
• Updates RPCv2 integration, tests, and SQLite dependency.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
go.mod	Updates SQLite dependency.
go.sum	Updates SQLite checksums.
internal/store/event.go	Defines view-based scanner API.
internal/store/event_match.go	Adds shared event matching logic.
rpcv2/stores/event/match.go	Reuses shared matcher types and functions.
rpcv2/stores/event/match_test.go	Updates matcher tests.
rpcv2/stores/event/extract_test.go	Adapts extraction tests to views.
rpcv2/eventsapi/get_events_v2.go	Builds responses from event views.
rpcv2/eventsapi/get_events_v1.go	Delegates shared filtering and conversion.
rpcv1/sqlitedb/migration_test.go	Updates scanner callback signature.
rpcv1/sqlitedb/event.go	Scans stored events as views.
rpcv1/sqlitedb/event_test.go	Updates view-based scanner tests.
methods/get_events.go	Uses shared filters and view-based responses.

Note

Copilot is running an experiment and ran this review at Balanced.

---

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

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Note

Copilot is running an experiment and ran this review at Balanced.

[tamirms]

Approving. All six earlier comments are addressed in the code rather than only in replies: the matcher is beside the handler, the shared file is gone and rpcv2 is back to base, the type fold is restored, the error code is fixed with the captured-error pattern, and the renderer reads through the located field bundles. I checked that those bundles trim every field the new code takes bytes from, so the rendering is still byte-identical.

The three comments below change no behavior, so they do not need another review round. Merge once they are addressed or consciously skipped. The first is the largest of them and can equally land here or fold in with #1004, which takes the ledger reads the same way.

[tamirms]

The awkwardness in the handler's error path traces back to this shape rather than to the wrapping around it. A callback cannot return to the function that set it up, so everything it produces escapes through captured state: results, and now procErr as well, which is why the error travels out twice and the handler discards the copy the reader hands back. This supersedes the captured-variable fix I pointed at on the error-code thread, since that pattern is what makes the error travel twice.

An iterator removes the cause, and it is the shape the rest of the tree already uses for this job, including the rpcv2 events reader, ScanLedgers and the catalog scans.

// ScannedEvent is one row of an event scan. Event aliases the backend's row
// buffer and is valid only inside the loop body that received it, so copy
// whatever outlives that body. TxHash is already a copy and is free to keep.
type ScannedEvent struct {
	Event           xdr.DiagnosticEventView
	Cursor          protocol.Cursor
	LedgerCloseTime int64
	TxHash          xdr.Hash
}

type EventReader interface {
	// GetEvents yields the events in cursorRange that pass the coarse
	// filters, in ascending cursor order. Break to stop early. A non-nil
	// error ends the stream and the ScannedEvent beside it is zero.
	GetEvents(
		ctx context.Context,
		cursorRange protocol.CursorRange,
		contractIDs [][]byte,
		topics TopicFilters,
		eventTypes []int,
	) iter.Seq2[ScannedEvent, error]
}

The handler becomes an ordinary loop over ordinary locals:

for ev, err := range h.dbReader.GetEvents(ctx, cursorRange, contractIDs, topics, eventTypes) {
	if err != nil {
		return protocol.GetEventsResponse{}, &jrpc2.Error{Code: jrpc2.InvalidRequest, Message: err.Error()}
	}
	event, err := ev.Event.Event()
	if err != nil {
		return protocol.GetEventsResponse{}, errors.Wrap(err, "could not parse event")
	}
	head, err := eventHeader(event)
	if err != nil {
		return protocol.GetEventsResponse{}, errors.Wrap(err, "could not parse event")
	}
	if !filters.matchHeader(head) {
		continue
	}
	body, err := eventBody(head.v0)
	if err != nil {
		return protocol.GetEventsResponse{}, errors.Wrap(err, "could not parse event")
	}
	if !filters.match(head, body.topics) {
		continue
	}
	info, err := eventInfo(head, body, ev.Cursor,
		time.Unix(ev.LedgerCloseTime, 0).UTC().Format(time.RFC3339), ev.TxHash.HexString(), request.Format)
	if err != nil {
		return protocol.GetEventsResponse{}, errors.Wrap(err, "could not parse event")
	}
	results = append(results, info)
	if uint(len(results)) >= limit {
		break
	}
}

A render failure returns plainly, which is the code base gave it, so the join in the sqlite loop has nothing left to route around and procErr goes away.

On the sqlite side the body of the existing rows.Next() loop becomes a yield, the query and scan failures become yields of their own, and rows.Err() becomes the trailing one. defer rows.Close() still runs when the consumer breaks, and the borrow is unchanged, since the loop body runs inside yield before the next Next().

It also lets the loan sentence say something true. txHash is an xdr.Hash array conversion, so it is already a copy and safe to retain; only the event view aliases the row buffer.

Scope is one interface method, one implementation, one production caller and six test files.

The shape change is mechanism neutral: range-over-func compiles the loop body into a closure the iterator calls, so it is the same one indirect call per row that the callback is today. BenchmarkGetEvents and BenchmarkGetEventsTopicFilters are in this package if you want before and after numbers.

#1004 takes the ledger reads the same way and deletes readLedgerPage with its own procErr, so this is one direction rather than two separate asks.

[tamirms]

ContractEventTypeView.Value() rejects any discriminant outside the three enum members, and eventHeader reads the type through Fields() and Value(), so an unknown type cannot reach here and the "" is unreachable.

Returning (string, bool) and erroring on !ok would say that outright. It would also let checkResponseEventType in eventsapi stop re-deriving an enum check from a rendered name: today it reports stored event has type "" for a case that cannot happen, when its real job is refusing the diagnostic type, which is a perfectly valid name.

[tamirms]

Base sized this to the page, first make([]entry, 0, limit) and then make([]protocol.EventInfo, 0, len(found)), so a full page no longer regrows from empty. make([]protocol.EventInfo, 0, limit) restores that and is still non-nil, so an empty page still serializes as [].