soroban-rpc: Remove `getEvents.endLedger` param

[paulbellamy]

What problem does your feature solve?

In the getEvents method, endLedger and pagination.limit conflict somewhat, and having both is unnecessary.

What would you like to see?

We should remove endLedger, and just use startLedger combined with pagination.limit.

So that clients can detect when they are out-of-bounds, we should also make sure that startLedger errors if it is:

• < oldest ledger this node has stored
• > latestLedger

This is a very breaking change, so we'll need to update all the clients at the same time.

[tamirms]

this was implemented in #385 but I have not updated the clients yet

[leighmcculloch]

endLedger and pagination.limit conflict somewhat, and having both is unnecessary.

What is the pagination limit?

If I wish to iterate over all events between two ledgers, how do I work out what the pagination limit should be too get them all?

Secting a start ledger and page limit is not particularly intuitive to me. My application probably doesn't care how many pages there are.

[tamirms]

@leighmcculloch we'll set the max limit to 10,000 events.

You're right that if you strictly want events spanning 2 ledgers and nothing more than this API is not ideal (the client will need to discard any events that are not relevant). But, I think the more common use case is: "get me all events since X." For that use case, I think the API proposal is good.

[leighmcculloch]

If there are more than 10,000 events, will this mean if you try and get all events between two ledgers where more than 10,000 events occurred between them, it might appear like you've gotten all the events when in fact you've only gotten the events that fit in the first 10,000?

[tamirms]

@leighmcculloch in the old API there was a startLedger, endLedger, and limit. The scenario you are presenting is more problematic for the old API than the new API. Because in the old API, the client might assume that it has covered all the events between startLedger and endLedger when in reality the number of events in the range exceeds limit .

In both the old API and the current proposal, paging is designed like it is in horizon: you use the last element in the response as a cursor for the next request to obtain the next page of events.

In the new API you are only promised that you will receive up to limit events starting from startLedger. So you can only assume that you have covered range of ledgers when you start receiving events outside of your intended stopping point.

The scenario where I could see this new API being problematic is if the events you have obtained are less than the page limit. In that case, you have exhausted all events available in the node but you don't know what the ledger the node is synced to. So perhaps it would make sense to include a latestLedgerAvailable field in the response so the client knows the full range of ledgers contained by the node. Alternatively, we could follow what Horizon does which is to include Latest-Ledger header included in every response for all requests.

[tamirms]

@leighmcculloch btw the cursor param overrides startLedger to provide the location of the events where we start our search. The cursor can point to any event (e.g. it has more granularity than a single ledger which contains lots of events).

[tamirms]

js client: stellar/js-soroban-client#49
soroban-cli: #389