-
Notifications
You must be signed in to change notification settings - Fork 625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(eureka): Add gRPC query for Counterparty for a given client identifier #7120
feat(eureka): Add gRPC query for Counterparty for a given client identifier #7120
Conversation
thanks! is there additional work needed here before making this ready for review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I contributed to the PR, but giving me approval. Thanks, @vishal-kanna!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some minor comments, can push fixes for it soon
|
||
sdkCtx := sdk.UnwrapSDKContext(ctx) | ||
creator, found := q.GetCreator(sdkCtx, req.ClientId) | ||
if found { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these needed? we get a default of ""
either way?
proto/ibc/core/client/v1/query.proto
Outdated
// QueryClientRequest is the response type for the Query/Client RPC method | ||
message QueryClientResponse { | ||
string creator = 1; | ||
Counterparty counterparty = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for pointer?
testCases := []struct { | ||
msg string | ||
malleate func() | ||
errMsg string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we go with the expError
pattern instead?
cleaning these up! |
closes: #7116