Add cursor in GetEventResponse

[psheth9]

What

Add endLedger in GetEventResponse

Why

• This tells the client until what ledger events are being queried. e.g.: startLedger (inclusive) - endLedger (exclusive)
• currently getEvents are capped by 10K LedgerScanLimit which means you can query events for 10K ledger at maximum for a given request.

Fixes: #289 and #118

[Shaptic]

Shouldn't we add a cursor, instead? It will be more precise relative to the end ledger.

[psheth9]

I am more inclined towards endLedger for simplicity, cursor makes sense and is already there in eventInfo when we wan to paginate.

but this is for the case when no events are found for given filter and we need to tell clients that we looked thru startLedger to endLedger range so that client could just copy paste the endLedger to startLedger in follow up request.

We had a discussion in past around this here We got this suggestion to add endLedger from Product team/Jane.

[tamirms]

I think I agree with @Shaptic in that it would be more useful to have a field in the response which indicates what cursor value should be used so we can continue the search. Assume the search window is [request.StartLedger, request.EndLedger], consider the following possibilities:

1. there are more than 10k events in the search window and the response is full
2. there are less than 10k events in the search window and the search window is below the latest ledger in the soroban-rpc db
3. there are less than 10k events in the search window, however the search window exceeds the latest ledger in the soroban-rpc db

For case (1) you cannot use EndLedger in the response to continue the search. Instead, you must use the cursor value from the last event in the response.

For case (2) you can use EndLedger in the response to continue the search.

For case (3) you cannot use EndLedger in the response to continue the search and instead you should use the LatestLedger field from the response

[sreuland]

driving by, agree with the suggestions for encapsulating the notion of 'next' into a cursor on the response, as @tamirms highlights the complexity for a client to use endLedger correctly. GetEventResponse could provide the pre-computed 'next' cursor based on internal endLedger/scanLimit chunking, and the caller can just paste into its next GetEventRequest:

type GetEventsResponse struct {
	Events       []EventInfo `json:"events"`
	LatestLedger uint32      `json:"latestLedger"`
        Pagination  *PaginationOptions `json:"next,omitempty"`
}

if 'next' is absent, implies there are no more events in rpc db to scan at that time.

[psheth9]

Thank you for all the inputs @tamirms @sreuland

okay so we talked about this in planning and agreed on using cursor instead of endLedger. Also now with this task #118 (move pagination-token/cursor to root level) it actually kinda makes more sense to not introduce more fields for less confusion.

Building up on aforementioned cases:

1. there are more than 10k events in the search window and the response is full --> client can use cursor to paginate thru in next folllow up request. This is simple and same as existing flow. note: here cursor id is last populated event_id

2. No events found for given search window --> we will set the cursor_id to startLedger + LedgerScanLimit so that client can make a follow up request to fetch events in next search window. note: here cursor id is represents the start of the next search window

so all in all we are using cursor_id as normal pagination token and also as end of the search window based on the use cases.

@janewang Do you have any feedback/suggestions on this?
I will make sure to update the getEvents doc to reflect all this.

[janewang]

Hey @psheth9, thanks! Will this PR going in as part of Protocol 22? I think we may need to make update Lab UI for getEvents as well:

https://lab.stellar.org/endpoints/rpc/get-events?$=network$id=testnet&label=Testnet&horizonUrl=https:////horizon-testnet.stellar.org&rpcUrl=https:////soroban-testnet.stellar.org&passphrase=Test%20SDF%20Network%20/;%20September%202015;;

[mollykarcher]

@janewang it will but it is not yet "breaking". See context here - #118 - but pagingToken will still work for now, and we'll remove it in p23. So lab should migrate to cursor when it can.

[janewang]

Got it, thank you @mollykarcher 👍