Skip to content
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

Add MsgServer to interchain accounts controller #2026

Closed
8 of 11 tasks
damiannolan opened this issue Aug 17, 2022 · 3 comments
Closed
8 of 11 tasks

Add MsgServer to interchain accounts controller #2026

damiannolan opened this issue Aug 17, 2022 · 3 comments
Labels
27-interchain-accounts type: feature New features, sub-features or integrations type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone

Comments

@damiannolan
Copy link
Member

damiannolan commented Aug 17, 2022

Summary

Add a new MsgServer to 27-interchain-accounts which exposes two distinct rpc endpoints:

  • RegisterAccount
  • SubmitTx

Problem Definition

The current design implemented in 27-interchain-accounts mandates that a base application referred to as an authentication module is developed in order to provide custom authentication as an entrypoint to interchain accounts. In this approach an application developer is expected to compose their base application (authentication module) with the ICS27 controllerKeeper and leverage the exported APIs exposed for account registration and sending of interchain accounts transactions.

Since the release of cosmos-sdk v0.46.x, the new design of x/gov and x/group allow for arbitrary message passing via the cosmos-sdk's MsgServiceRouter. When a proposal is accepted it allows the state machine to execute an arbitrary number of sdk.Msgs. Since this functionality has been introduced it has become clear that x/gov and x/group act as authentication modules with respect to interchain accounts integration.

Proposal

Introduce a new MsgServer to 27-interchain-accounts/controller and incrementally move towards deprecation of public APIs.
This allows for existing app developers to simply change their code from a public API (RegisterInterchainAccount / SendTx) call to message passing, whereby one would compose their app with the cosmos-sdk MsgServiceRouter and retrieve the handler appropriately based on the message typeURL, as is done in many existing modules such as x/gov, x/group as well as 27-interchain-accounts/host. This provides a more standardised approach to integrating with interchain accounts.

RegisterAccount

Introduce a new rpc endpoint RegisterAccount. This rpc should provide the existing functionality of the exported RegisterInterchainAccount function in 27-interchain-accounts/controller.

service Msg {
  rpc RegisterAccount(MsgRegisterAccount) returns (MsgRegisterAccountResponse);
  ...
}

message MsgRegisterAccount {
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string owner                      = 1;
  string connection_id              = 2 [(gogoproto.moretags) = "yaml:\"connection_id\""];
  string version                    = 3;
}

message MsgRegisterAccountResponse {
  string channel_id = 1 [(gogoproto.moretags) = "yaml:\"channel_id\""];
}

SubmitTx

Introduce a new rpc endpoint SubmitTx. This rpc should provide the existing functionality of the exported SendTx function in 27-interchain-accounts/controller.

service Msg {
  ...
  rpc SubmitTx(MsgSubmitTx) returns (MsgSubmitTxResponse);
}

message MsgSubmitTx {
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string owner                            = 1;
  string connection_id                    = 2 [(gogoproto.moretags) = "yaml:\"connection_id\""];
  InterchainAccountPacketData packet_data =  3 [(gogoproto.moretags) = "yaml:\"packet_data\""];
  ibc.core.client.v1.Height timeout_height = 4 [(gogoproto.moretags) = "yaml:\"timeout_height\""];
  uint64 timeout_timestamp = 5 [(gogoproto.moretags) = "yaml:\"timeout_timestamp\""];
}

message MsgSubmitTxResponse {
  uint64 sequence = 1;
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@damiannolan damiannolan added 27-interchain-accounts type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. labels Aug 17, 2022
@crodriguezvega crodriguezvega moved this to Todo in ibc-go Aug 18, 2022
@crodriguezvega crodriguezvega added this to the v5.1.0 milestone Aug 18, 2022
@damiannolan damiannolan added the type: feature New features, sub-features or integrations label Aug 22, 2022
@damiannolan damiannolan added the needs discussion Issues that need discussion before they can be worked on label Aug 24, 2022
@damiannolan
Copy link
Member Author

damiannolan commented Aug 24, 2022

Requires discussion regarding usage of google.protobuf.Any in MsgSubmitTx. This requires that the message type submitted for execution on the host chain is registered on the controller chain interface registry.
The obvious solution is to use the InterchainAccountPacketData type directly, however this may be challenging from a ux perspective as the message would need to be constructed and proto3 encoded to []byte off chain.

Edit: discussed with @colin-axner and decided the best option is to go with InterchainAccountPacketData in favour of Anys for the payload in SubmitTx.
It's not a good design or sustainable to force chains to register msg types of various counterparties to allow nested Anys to be resolved. While its tricky from a client UX perspective wrt to CLI's, it would likely be a non-issue for use-cases which are actually fronted by something like a client web application, where they could handle encoding of different msg types which belong to many chains.

More context in this thread: #2086 (comment)

@damiannolan damiannolan removed the needs discussion Issues that need discussion before they can be worked on label Aug 25, 2022
@colin-axner colin-axner moved this from Todo to In progress in ibc-go Aug 25, 2022
@colin-axner colin-axner moved this from In progress to In review in ibc-go Aug 31, 2022
@robert-zaremba
Copy link

Great issue. Using Msg routing is the right approach. BTW, x/authz is also using this mechanism.

@robert-zaremba
Copy link

At first glance it makes sense to register messages in the controller chain for Any.

However, It may create burden if there is one chain to control many (it would need to register all custom messages and keep track with versions).

Repository owner moved this from In review to Done in ibc-go Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
27-interchain-accounts type: feature New features, sub-features or integrations type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
Archived in project
Development

No branches or pull requests

3 participants