feat: rpc query service for returning the UpgradeSequence stored in state for a specific port/channel pair #3360
Conversation
…/1618-chan-upgrade-init
…datebasic, pull in subset ordering suggestions and adding tests
| // QueryUpgradeSequenceResponse is the response type for the QueryUpgradeSequence RPC method | ||
| message QueryUpgradeSequenceResponse { | ||
| uint64 upgrade_sequence = 1; | ||
| bytes proof = 2; |
There was a problem hiding this comment.
do we need the proof in the response proto? i see it in the NextSequenceReceive query but it seems to always be set to nil
There was a problem hiding this comment.
an unrelated question: do we need the cli commands for this query as well?
There was a problem hiding this comment.
do we need the proof in the response proto? i see it in the NextSequenceReceive query but it seems to always be set to nil
I believe the reason is that there is not support in SDK/Comet to generate proofs for a given key/value . @colin-axner opened this issue in the SDK a while back.
an unrelated question: do we need the cli commands for this query as well?
I think we have CLIs for all gRPC, soI would say that we also add it for consistency? What others think?
There was a problem hiding this comment.
IIRC some of the channel CLIs use a --prove flag and based off that flag an ABCI query can be done directly to tendermint rpc.
Unsure if there's any consistency to which ones contain the --prove flag and which do not. Haven't checked in depth. You may as well go ahead and add it!
There was a problem hiding this comment.
I believe the reason is that there is not support in SDK/Comet to generate proofs for a given key/value . @colin-axner opened cosmos/cosmos-sdk#7036 in the SDK a while back.
yes, I remember this issue from the relayers having to always query the tendermint rpc instead of being able to query proofs from the grpc endpoint. I guess the question is if we want to plan for a (possible?) future of a proof returned via grpc by still putting in the proof field in the response body, even if now it will always return nil, or just remove that proof field.
EDIT i see the cli util implemented which handles the --prove flag, i'll do it that way then.
charleenfei
requested review from
colin-axner,
AdityaSripal and
damiannolan
as code owners
| // QueryUpgradeSequenceResponse is the response type for the QueryUpgradeSequence RPC method | ||
| message QueryUpgradeSequenceResponse { | ||
| uint64 upgrade_sequence = 1; | ||
| bytes proof = 2; |
There was a problem hiding this comment.
do we need the proof in the response proto? i see it in the NextSequenceReceive query but it seems to always be set to nil
I believe the reason is that there is not support in SDK/Comet to generate proofs for a given key/value . @colin-axner opened this issue in the SDK a while back.
an unrelated question: do we need the cli commands for this query as well?
I think we have CLIs for all gRPC, soI would say that we also add it for consistency? What others think?
Description
closes: #1902
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.