Missing pagination for some bulk queries

[ancazamfir]

Summary

While looking at some pagination issues in hermes queries (informalsystems/ibc-rs#811) found that there is no pagination field for the following queries:
QueryClientConnectionsRequest, QueryUnreceivedPacketsRequest , QueryUnreceivedAcksRequest.

Problem Definition

Proposal

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[ancazamfir]

Actually probably not required for QueryUnreceivedPacketsRequest and QueryUnreceivedAcksRequest as we query for specific sequences.

[crodriguezvega]

@ancazamfir the lack of pagination in QueryClientConnectionsRequest, is still an issue for hermes?

I ask because I was looking a bit into it and I think that it will not be so straightforward to add pagination to it. I will try to explain...

I think the regular way of adding pagination to the gRPC endpoint using SDK's machinery would be something like this:

clientConnectionPaths := []string{}
store := prefix.NewStore(ctx.KVStore(q.storeKey), []byte(host.ClientConnectionsKey(req.ClientId)))
pageRes, err := query.Paginate(store, req.Pagination, func(key, value []byte) error {
	var result types.ClientPaths
	if err := q.cdc.Unmarshal(value, &result); err != nil {
		return err
	}

	clientConnectionPaths = append(clientConnectionPaths, result.Paths...)
	return nil
})
if err != nil {
	return nil, err
}

The problem is that the connection paths for a given client ID are all stored as a []string in the key clients/<client-id>/connections, so when doing the query.Paginate there's always just one page returned that contains all the connection paths for that client, since there's nothing to paginate over on the provided prefix ([]byte(host.ClientConnectionsKey(req.ClientId))).

Alternatively we could get all the connection paths as it is done right now and do the pagination ourselves, but before building something so specific for this query, I would like to know if there's really a strong need for pagination in this query.

cc @colin-axner to double check my reasoning here when he has some time or provide alternatives ideas.

[colin-axner]

That seems correct to me. The issue is that the request is to paginate the value returned, not the key (there's only one key). I'd assume this likely isn't a problem since there should only be a handful of connections per client

[ancazamfir]

It's been a long time and can't recall the details. I was doing local testing with high scale numbers but can't recall if anything was breaking and how it was breaking. We can do the tests again.
Are you asking in order to close the issue?

[crodriguezvega]

Are you asking in order to close the issue?

Yes, that's basically the reason. I can understand this could be a problem in stress-testing scenarios if you add lots of connections on top of the client, but if this is not an issue that would likely be hit in real scenarios, then I would consider closing the issue. It can be open anyway again if needed!

[crodriguezvega]

Closing for now since there's no action to take at the moment. We will reopen if needed.