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

Define IBC interface #147

Closed
ethanfrey opened this issue Feb 20, 2020 · 12 comments · Fixed by #884
Closed

Define IBC interface #147

ethanfrey opened this issue Feb 20, 2020 · 12 comments · Fixed by #884

Comments

@ethanfrey
Copy link
Member

With a stable IBC approaching (and the spec being finalized), it is a good time to begin specifying how contracts work with IBC. The idea is to make a design that is compatible with the full spec, with all security guarantees, but makes a number of assumptions in x/wasm to provide a simple but useful interface for the contracts.

The basic ideas from IBC include Client and Connection which are established between 2 blockchains - x/wasm doesn't touch these, it can only reuse existing connections, assuming a client passes in the relevant id.

There is the concept of Port, which a module must bind to be able to send/receive messages. These are arbitrary strings on a first come, first serve basis. To avoid spamming by contracts, x/wasm will check if the contract is IBC-enabled (it exports a few extra functions), and if so, will bind a port for that contract when it is instantiated of the form wasm:cosmos1q4djygw....

Then there is the concept of a Channel. A channel is established between one module on one chain and another on another chain. It consists basically of A: (port, connection) <-> B: (port, connection). Creating a channel involves a 3 step handshake. We need to allow cosmwasm contracts to establish channels, but ideally with one message/call (and let x/wasm handle the handshake for them).

Once a channel is established, it can be referenced inside the contract as (remote port, remote connection_id) which is the universal id of the counterparty. If this differs than the normal representation, then x/wasm can maintain a lookup table from channel id to this tuple.


In general, IBC is a remote function call. Imagine some method on the remote side, f(Caller, T) -> Result<U>. Where T and U are some serializable data structures, which depend on the protocol being used. (Same as HandleMsg, QueryMsg, etc).

To send: we add another type to CosmosMsg as a valid return value:

enum CosmosMsg{
  IBCCall{
    dest: struct {
      port: string,
      connection: string,
     },
     msg_id: string,
     protocol: string,
     payload: Base64,
  }
}

I think we should define the expected protocol somewhere. In order to properly parse the payload, we must ensure that both sides are speaking the same language. I do not need some complex two-way protocol negotiation, but some way to mark which protocol we expect them to speak (eg. ICS20) and if they do not know the string, they can reject the connection (rather than possibly parsing the payload incorrectly).

msg_id is an arbitrary unique string set by the cosmwasm contract that is not sent, but used by x/wasm when it receives the response (to allow the contract to maintain a lookup table of pending responses without worrying about the internal ibc numbering system).

The contract must then export two other functions that will be called at various steps in the lifetime of the packet.

ibc_receive(Extern, Params, IBCCall<IBCMsg>) -> Result<IBCResponse>

This is called on the receiving end of the call, after a relayed brings a proof of the committed IBCCall message. IBCMsg is the contract-specific struct to deserialize the binary payload, just like HandleMsg, etc.

ibc_ack(Extern, Params, msg_id, Result<IBCResponse>) -> Result<()>

After the message has been properly processed on the receiving side, an acknowledgement is returned to the server containing either a success message (in a protocol-specific format) or an error, along with the msg_id from the original call. This allows the contract to update it's cache of "pending transactions" and fully commit or revert the transaction.

@cwgoes
Copy link

cwgoes commented Feb 21, 2020

Generally, this makes sense to me - a few comments:

Then there is the concept of a Channel. A channel is established between one module on one chain and another on another chain. It consists basically of A: (port, connection) <-> B: (port, connection). Creating a channel involves a 3 step handshake. We need to allow cosmwasm contracts to establish channels, but ideally with one message/call (and let x/wasm handle the handshake for them).

Just as a point of clarity, channels are necessarily associated with a single connection, but they aren't uniquely denoted by the pair of ports and a connection - they have individual identifiers - the unique tuple across the combined state of both chains is ( (portA, channelIdentifierA), (portB, channelIdentifierB) ).

I think we should define the expected protocol somewhere. In order to properly parse the payload, we must ensure that both sides are speaking the same language. I do not need some complex two-way protocol negotiation, but some way to mark which protocol we expect them to speak (eg. ICS20) and if they do not know the string, they can reject the connection (rather than possibly parsing the payload incorrectly).

Hmm. In general I agree, although I think we will also want "dynamic" WASM contracts - namely, contracts which can speak IBC protocols that may not be fixed prior to deployment of wasmd, so it may not be possible for the state machine outside of WASM code to check that the contract(s) are in compliance.

msg_id is an arbitrary unique string set by the cosmwasm contract that is not sent, but used by x/wasm when it receives the response (to allow the contract to maintain a lookup table of pending responses without worrying about the internal ibc numbering system).

This is possibly not necessary since the IBC message sequence is queryable by SDK application code & each channel will be associated with a unique contract, but it's fine if you need a separate identifier for some x/wasm internal message routing reason.

After the message has been properly processed on the receiving side, an acknowledgement is returned to the server containing either a success message (in a protocol-specific format) or an error, along with the msg_id from the original call. This allows the contract to update it's cache of "pending transactions" and fully commit or revert the transaction.

Aye, exactly!

@ethanfrey
Copy link
Member Author

Thank you for the detailed reply, and it all makes sense. As to protocol if we use the version field in the handshake, then that should be a way to dynamically inform the other side of the expected protocol.

Just as a point of clarity, channels are necessarily associated with a single connection, but they aren't uniquely denoted by the pair of ports and a connection - they have individual identifiers - the unique tuple across the combined state of both chains is ( (portA, channelIdentifierA), (portB, channelIdentifierB) ).

A channel consists of a port and a connection, right? Can the same channelIdentifierA connect to multiple ports of chain A? And if not what is the use case for multiple channels between portA, connA and portB, connB? I can only think of a high priority and a low priority channel between a pair of contracts. But I am sure I am missing something

@cwgoes
Copy link

cwgoes commented Feb 21, 2020

Thank you for the detailed reply, and it all makes sense. As to protocol if we use the version field in the handshake, then that should be a way to dynamically inform the other side of the expected protocol.

Yes, and the other end can select a version it supports or reject the handshake if none are supported.

A channel consists of a port and a connection, right? Can the same channelIdentifierA connect to multiple ports of chain A? And if not what is the use case for multiple channels between portA, connA and portB, connB? I can only think of a high priority and a low priority channel between a pair of contracts. But I am sure I am missing something

A channel is identified by a port identifier and a channel identifier, and is associated with one connection. A given channel & port identifier on chain A are associated with exactly one channel & port identifier on chain B. Multiple channels for the same connection can be used for regular multi-module communications (e.g. there are lots of contracts on chain A which want to talk to lots of contracts on chain B - there would be one connection but lots of channels, one per pair of contracts) - it's also possible to multiplex on a single channel, but the spec allows many channels-per-connection so it can be done at this layer. In the future connections could also have some sort of flow control or more complex ordering dependence - see https://github.com/cosmos/ics/issues/133, https://github.com/cosmos/ics/issues/131 (both "IBC 2.0" features).

@ethanfrey
Copy link
Member Author

For reference (links behind Chris's explanations), look at ICS 4.

In particular, it defines the Packet as:

interface Packet {
  sequence: uint64
  timeoutHeight: uint64
  sourcePort: Identifier
  sourceChannel: Identifier
  destPort: Identifier
  destChannel: Identifier
  data: bytes
}

({dest,source}{Port,Channel} being the unique identifiers, sequence could serve as msg_id, and data is the serialized packet info (T above).

@ethanfrey
Copy link
Member Author

ethanfrey commented Feb 21, 2020

Versioning is defined here to be on a per-channel basis and can be used for protocol negotiation. I had assumed we could simplify the ICS potential for CosmWasm such that (portA, connA), (portB, connB) is unique, assuming that "one channel per port + connection". Chris says this we can allow multiple, and I do see the choice of multiple communication protocols as a valid reason. Maybe we can make the following unique (to provide a simpler interface to contracts): (portA, connA), (portB, connB), version. Then from the contract A's view, it can define (connA, portB, version) and provide enough info for x/wasm to unique define the desired channel (and reuse it or create it if needed).

The channel state lifecycle diagram explains this process. What is unclear is where we have hooks that x/wasm can plug in to, to provide some sort of version negotiation. The channel open handshake pseudocode doesn't provide much except enforcing that both sides have the same version string. But where does it come from? (I guess this is more and SDK-implementation issue than a spec issue)

@ethanfrey
Copy link
Member Author

This comment does make it seem that version is geared more to the IBC implementation (IBCv1, IBCv2), rather than the data packet format. So maybe it does makes sense to add another field for that, and let version determine the IBC/handshake/channel protocol version, not the inter-module message flow. That said, we could leave this as is, and just create a convention that the first packets sent on a channel are version information in some "well known format". For example:

interface ChannelProtocolRequest {
    desiredProtocol string
    supportedProtocols string[]
}

type ChannelProtocolResponse = ChannelProtocolMatch | ChannelProtocolError;

interface ChannelProtocolMatch {
    acceptedProtocol: string
}

interface ChannelProtocolError {
    error: "no compatible protocol
}

If the first packet from the initiating side (that calls ChannelInit) is always ChannelProtocolRequest and the first packet from the receiving side is always ChannelProtocolResponse, this provides a consistent and predictable application-level protocol handshake without any modifications to ICS4. It would not be enforced, but if all major applications use this convention, we can assume it in the applications.

@cwgoes if you like this idea, I will make a proper Issue/PR on ICS (this is not CosmWasm-specific)

@cwgoes
Copy link

cwgoes commented Feb 21, 2020

The channel state lifecycle diagram explains this process. What is unclear is where we have hooks that x/wasm can plug in to, to provide some sort of version negotiation. The channel open handshake pseudocode doesn't provide much except enforcing that both sides have the same version string. But where does it come from? (I guess this is more and SDK-implementation issue than a spec issue)

See ICS 26 (module callback interface) for the list of hooks & their signatures.

It may make sense to separate "IBC protocol" version negotiation and "application" version negotiation - however, I am not sure that the former is necessary at all in the channel handshake, it could happen earlier (e.g. with the connection or even with the client).

@ethanfrey
Copy link
Member Author

After discussing offline, we agreed that ibc protocol level lives on a different level, and connection version is for application-protocol version.

There will be a standardized ics20 protocol name: cosmos/ibc#381 as a starting point, and application developers can follow a similar convention set down by this core module.

@dtribble
Copy link

My characterization of dIBC is I think a simpler starting point. I was not proposing to add negotiation and versioning and such (though those are all valuable). Instead I was focused just on deploying new code on an existing chain to handle a new App protocol. I assume in this that the specific App protocol is known to both ends (based on the ports they are connecting to) rather than being a matter of negotiation. The "dynamic" is just about the ability to expand the App protocols processed by the chain by deploying support for it dynamically.

I also think that the protocols defined this way ought to be understandable to e.g., Go modules that get deployed in a chain upgrade later. Thus, they should be amenable to a static deployment.

Thus though we may want protocol negotiation, we should avoid implementing/encouraging protocols that put that in-band (if we can) unless they do so in a way that is compatible with e.g., a future implementation of those App packet types in Go or Rust. Since those cannot handle parsing payloads of types they weren't compiled with, that puts some constraints on the protocols we propose.

@webmaster128
Copy link
Member

Can this be considered done with #692 closed?

@ethanfrey
Copy link
Member Author

ethanfrey commented Feb 2, 2021

The code is well-done. I should add a usage doc. Thanks for reminding me.

Also, I should check with the ICS spec. If they will eventually allow contracts/modules to participate in version negotiation (rather than asking the relayer to set that), then we may need to expose some other return values.

Also, see CosmWasm/wasmd#398 for the documentation work

@cwgoes
Copy link

cwgoes commented Feb 2, 2021

Also, I should check with the ICS spec. If they will eventually allow contracts/modules to participate in version negotiation (rather than asking the relayer to set that), then we may need to expose some other return values.

Whatever changes are necessary to enable this, if any, shouldn't be controversial I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants