-
Notifications
You must be signed in to change notification settings - Fork 585
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: rename RegisterCounterpartyAddress rpc to RegisterCounterpartyPayee #1513
Changes from all commits
6e65707
6ef0b44
cea4938
679e0ee
6121ec0
ac2dd48
473b113
7af30a6
49ce2f6
d8511e4
88f01c2
e865222
bdb821a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ const ( | |
// NewRegisterPayeeCmd returns the command to create a MsgRegisterPayee | ||
func NewRegisterPayeeCmd() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "register-payee [port-id] [channel-id] [relayer-address] [payee-address] ", | ||
Use: "register-payee [port-id] [channel-id] [relayer] [payee] ", | ||
Short: "Register a payee on a given channel.", | ||
Long: strings.TrimSpace(`Register a payee address on a given channel.`), | ||
Example: fmt.Sprintf("%s tx ibc-fee register-payee transfer channel-0 cosmos1rsp837a4kvtgp2m4uqzdge0zzu6efqgucm0qdh cosmos153lf4zntqt33a4v0sm5cytrxyqn78q7kz8j8x5", version.AppName), | ||
|
@@ -47,6 +47,31 @@ func NewRegisterPayeeCmd() *cobra.Command { | |
return cmd | ||
} | ||
|
||
// NewRegisterCounterpartyPayeeCmd returns the command to create a MsgRegisterCounterpartyPayee | ||
func NewRegisterCounterpartyPayeeCmd() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "register-counterparty-payee [port-id] [channel-id] [relayer] [counterparty-payee] ", | ||
Short: "Register a counterparty payee address on a given channel.", | ||
Long: strings.TrimSpace(`Register a counterparty payee address on a given channel.`), | ||
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. [nit] unnecessary 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 left this in from existing code and I think there's a good few of them around. Maybe we could open a follow up PR to remove them if you want, some general housekeeping. |
||
Example: fmt.Sprintf("%s tx ibc-fee register-counterparty-payee transfer channel-0 cosmos1rsp837a4kvtgp2m4uqzdge0zzu6efqgucm0qdh osmo1v5y0tz01llxzf4c2afml8s3awue0ymju22wxx2", version.AppName), | ||
Args: cobra.ExactArgs(4), | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
clientCtx, err := client.GetClientTxContext(cmd) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
msg := types.NewMsgRegisterCounterpartyPayee(args[0], args[1], args[2], args[3]) | ||
|
||
return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) | ||
}, | ||
} | ||
|
||
flags.AddTxFlagsToCmd(cmd) | ||
|
||
return cmd | ||
} | ||
|
||
// NewPayPacketFeeAsyncTxCmd returns the command to create a MsgPayPacketFeeAsync | ||
func NewPayPacketFeeAsyncTxCmd() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
|
@@ -122,28 +147,3 @@ func NewPayPacketFeeAsyncTxCmd() *cobra.Command { | |
|
||
return cmd | ||
} | ||
|
||
// NewRegisterCounterpartyAddressCmd returns the command to create a MsgRegisterCounterpartyAddress | ||
func NewRegisterCounterpartyAddressCmd() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "register-counterparty [port-id] [channel-id] [address] [counterparty-address] ", | ||
Short: "Register a counterparty relayer address on a given channel.", | ||
Long: strings.TrimSpace(`Register a counterparty relayer address on a given channel.`), | ||
Example: fmt.Sprintf("%s tx ibc-fee register-counterparty transfer channel-0 cosmos1rsp837a4kvtgp2m4uqzdge0zzu6efqgucm0qdh osmo1v5y0tz01llxzf4c2afml8s3awue0ymju22wxx2", version.AppName), | ||
Args: cobra.ExactArgs(4), | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
clientCtx, err := client.GetClientTxContext(cmd) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
msg := types.NewMsgRegisterCounterpartyAddress(args[0], args[1], args[2], args[3]) | ||
|
||
return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) | ||
}, | ||
} | ||
|
||
flags.AddTxFlagsToCmd(cmd) | ||
|
||
return cmd | ||
} |
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.
This ensures they will be properly compensated for forward relaying since the destination chain must include the registered counterparty payee address in the acknowledgement.
This is not complete true with the introduction of the the MsgRegisterPayee, right? Because the payee that you register could get the fees even if the counterparty address is not registered. An operator could decide not to use the counterparty payee at all and only the payee, right?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've updated
must
tomay
and included an additional note:// A registered payee on the source chain will always override a registered counterparty payee.
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.
Sorry, @damiannolan, I think my comment above is not correct. I just realized while I was writing this comment on the other PR. I think you still need to register the counterparty address, otherwise we wouldn't know on the source chain to which payee to pay out the fees. The counterparty address may or may not have a payee registered.
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.
After the conversation this morning in the planning call, I think we need to update this comment again, right?
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.
Yep, done! Feel free to suggest changes