-
Notifications
You must be signed in to change notification settings - Fork 618
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
chore: MsgUpdateClient rename header to client_message #1316
chore: MsgUpdateClient rename header to client_message #1316
Conversation
// header to update the light client | ||
google.protobuf.Any header = 2; | ||
// client message to update the light client | ||
google.protobuf.Any client_message = 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.
I'm unsure if this was forgotten about or if this was intended to be left as header
. If not required we can close this PR or take the deprecation notices to another PR.
My only concern here is that relayers would be required to support both field names simultaneous depending on what version a chain and it's counterparty are running. Afaik hermes queries chains to determine ibc-go versions already but I am not sure if they have had to support such a scenario yet.
Despite this I think we should still be allowed to freely rename such fields for major version releases.
Curious to hear the thoughts of the team! :)
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.
Good question... If needed I can check with Adi and Justin.
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 believe updating the naming should be fine. But we should check with relayers
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.
ACK. Nice!
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.
Yurt
* updating MsgUpdateClient to use client_message field in favour of header * updating clis, field naming, and adding deprecation notices
Description
header
field toclient_message
onMsgUpdateClient
closes: #XXXX
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/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes