fix: increase pagination limit, return chain tip if exceeded

[SantiagoPittella]

Currently, starting the node is failing due to max iterations being reached. This was caused by the store returning the block_from instead of the chain tip if it was exceeded, and missing validations of the block range.

I added the same checks that we use for the other paginated endpoints, increased the limit in the client, we now return the chain tip if the reached block number exceeds it, and updated the check to be greater or equal instead of just equal to the chain tip.

[Mirko-von-Leipzig]

iiuc this is handling a race condition and we're essentially lying about the chain_tip since last_block_included exceeds it?

What should be happening is that chain_tip and the result are fetched within the same database transaction so that they're atomic.

In general the API isn't really a good match for what we use it for - there isn't really a need to have a block range, but rather just a starting block. But since we're (hopefully) reworking this soon, let's just patch it by doing an extra call after we've fetched the data.

Suggested change

	if last_block_included > chain_tip {
	last_block_included = chain_tip;
	}
	chain tip = self.state.latest_block_num().await;

[SantiagoPittella]

iiuc this is handling a race condition and we're essentially lying about the chain_tip since last_block_included exceeds it?

We are always returning the proper chain tip. A few lines under that we have:

        Ok(Response::new(proto::store::NetworkAccountIdList {
            account_ids,
            pagination_info: Some(proto::rpc::PaginationInfo {
                chain_tip: chain_tip.as_u32(),
                block_num: last_block_included.as_u32(),
            }),
        }))

The issue was that in the case that one page was able to return all the entries, the last block included was set to be the one sent by the user, which could be higher than the chain tip. Basically, we were saying that we included blocks past the tip, which is not true since those not even exists. So I add that step to truncate the last included block to the chain tip.

What should be happening is that chain_tip and the result are fetched within the same database transaction so that they're atomic.

About the atomicity, you are right on this, a new block can appear between the initial chain tip and the account IDs retrieval. I added the new fetch of the chain tip as you mentioned. Though it makes me think that there is a case when between a function the account IDs and the last chain tip fetch, a new block is created so the chain tip can be last_included_block + N, causing the client to be always out of sync and retrying. This case will be solved when using a stream too.

[Mirko-von-Leipzig]

Ah I see so underneath is also a weird behaviour etc. Pagination is a pain 🙃

I think this is good enough for now. But definitely in this internal API we always want to fetch all accounts sequentially until we reach the end. So a streaming API works, as does a pagination without an end point i.e. just a continuation token set to something like rowid.