-
Notifications
You must be signed in to change notification settings - Fork 684
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
refactor: change counterparty structure. #7328
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,12 @@ import "ibc/core/commitment/v2/commitment.proto"; | |
|
||
// Counterparty defines the counterparty for a light client to implement IBC eureka protocol | ||
message Counterparty { | ||
// the client identifier of the counterparty chain | ||
// the client identifier of the light client representing the counterparty chain | ||
string client_id = 1; | ||
// the merkle path that all ICS24 paths will be stored under | ||
ibc.core.commitment.v2.MerklePath merkle_path_prefix = 2 [(gogoproto.nullable) = false]; | ||
// the counterparty identifier that must be used by the packet | ||
string counterparty_channel = 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would definitely make sure any ids are suffixed with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doing this rename atm |
||
// the key path used to store packet flow messages that the counterparty | ||
// will use to send to us. In backwards compatible cases, we will append the channelID and sequence in order to create | ||
// the final path. | ||
ibc.core.commitment.v2.MerklePath merkle_path_prefix = 3 [(gogoproto.nullable) = false]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was there a reason that |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,8 +25,8 @@ message MsgProvideCounterparty { | |
|
||
option (gogoproto.goproto_getters) = false; | ||
|
||
// client unique identifier | ||
string client_id = 1; | ||
// unique identifier we will use to write all packet messages sent to counterparty | ||
string channel_id = 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Colin suggested There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think talking about source and dest makes sense outside the context of a packet There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that makes sense |
||
// counterparty client | ||
Counterparty counterparty = 2 [(gogoproto.nullable) = false]; | ||
// signer address | ||
|
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.
My preference/recommendation would be to use the
authority
naming rather than "creator"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.
wouldn't that be more confusing? If used I'd assume the module's authority address should be what we're comparing with after (instead of
msg.Signer
). Either case, happy if issue is opened and discussed afterwards?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.
maybe its confusing intertwine terminology used for module permissions and client creation.
The way I thought about it was:
msg.Signer
creates a client, thus they have the authority to bind a counterparty to it.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.e.
SetClientAuthority(ctx, clientID, msg.Signer)
&&GetClientAuthority(ctx, clientID)
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 issue with me, if your pref is strong, I say we open issue and slam it through in follow up.
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 not quite sure how strong my pref is! 😆
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 find creator clearer personally. but maybe that's my own mental model. Don't have a strong preference