feat: add BlockRange and PaginationInfo messages#1205
Conversation
SantiagoPittella
changed the title
BlockRange and PaginationInfo messages
| #[prost(fixed32, optional, tag = "2")] | ||
| pub block_to: ::core::option::Option<u32>, | ||
| } | ||
| /// Represents pagination information. |
There was a problem hiding this comment.
nit: it is pagination across a single axis of blocks, we might have different pagination axis/multiple so we should be specific in the comment
There was a problem hiding this comment.
Currently it is the only axis, and I'm not aware of a plan to add another. I'm updating docs to better reflect this.
SantiagoPittella
requested review from
Mirko-von-Leipzig,
bobbinth and
igamigo
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left one small comment inline.
Also, not for this PR, but should we also make similar changes to the SyncNotes messages?
proto/proto/store/rpc.proto
Outdated
| // To request the next chunk, the client should use the `block_num` from the previous response | ||
| // as the `block_from` for the next request. | ||
| message PaginationInfo { |
There was a problem hiding this comment.
This is a bit ambiguous because it doesn't use the +1 - so, it is not clear whether we should use just block_num from the previous response or bock_num + 1. I would rewrite this to clarify.
Also, nit: let's add a newline after line 423.
SantiagoPittella
deleted the
santiagopittella-implement-block-range-message
branch
4 participants
closes #1204