ICS 027: Interchain account #278

Thunnini · 2019-09-26T13:18:01Z

Changes

Use acknowledge packet instead of result packet.
Module saves the interchain account's addresses in its state.

mossid

Had the first iteration on the specification, thanks for proposing the interchain accounts. Comments are mostly on the chain-specific parts. Multimsg transaction is also Cosmos specific concept, will look into that and review.

mossid · 2019-10-14T12:13:40Z

spec/ics-027-interchain-account/README.md

+    // because attackers can distrupt to create an ibc managed account
+    // by sending some assets to estimated address in advance.
+    // And IBC managed account has no public key, but its sequence is 1.
+    // It can be mark for Interchain account, becuase normal account can't be sequence 1 without publish public key.


This assumption seems a bit Cosmos specific. Can we change address = sha256(path + packet.salt) to arbitrary address generation logic, so the receiving chain can generate a non-conflicting address? In Cosmos, the receiving chain can generate a new address with that hashing logic, with different bech32 prefix, and returns it, and in other chains, for example in Ethereum, the receiving chain can generate a new contract that works as a proxy for the origin chain and returns its address.

Agreed, we should avoid Cosmos SDK specific assumptions.

I agree. So, is it better to change the determining address login to function interface? For example, make function typetype CreateAddress = funtion(packet:Packet):Uint8Array and use it for determining address. But, I think there is no need to consider bech32, because the address is just byte address and we can interact by byte address in the codebase.

mossid · 2019-10-14T12:14:14Z

spec/ics-027-interchain-account/README.md

+    // It can be mark for Interchain account, becuase normal account can't be sequence 1 without publish public key.
+    account = accountKeeper.getAccount(account)
+    if (account != null) {
+      abortTransactionUnless(account.sequence === 1 && account.pubKey == null)


The sequence an pubkey field of the account is Cosmos specific

I think that sequence (or nonce) and a public key are very general in the blockchain. So, I think using sequence and public key is not Cosmos specific. But, in the above discussion, I agree it is better to make account keeper and account as the interface so, I think, it is good to define the account interface that has the sequence (or nonce) and public key and use those variables from the interface.

mossid · 2019-10-14T12:18:01Z

spec/ics-027-interchain-account/README.md

+    addressesRegisteredChannel[signer] = path
+
+    // set account's sequence to 1
+    accountKeeper.setAccount(address, {


Also accountKeeper seems Cosmos specific

mossid · 2019-10-14T12:18:51Z

spec/ics-027-interchain-account/README.md

+    // by sending some assets to estimated address in advance.
+    // And IBC managed account has no public key, but its sequence is 1.
+    // It can be mark for Interchain account, becuase normal account can't be sequence 1 without publish public key.
+    account = accountKeeper.getAccount(account)


Also accountKeeper seems Cosmos specific. As an alternative you declare an interface named accountKeeper with a set of functions with some constraints(which will be inspired from Cosmos' accountKeeper design, but works chain agnostic)

Here is where I would like to see a runTx function.

mossid · 2019-10-14T12:21:56Z

spec/ics-027-interchain-account/README.md

+      ...,
+    })
+
+    // Return acknowledgement byte as 0x0 if account creation succeeds.


If the address generation works arbitrary(as commented above) the return ack should include generated address

mossid · 2019-10-14T12:23:00Z

spec/ics-027-interchain-account/README.md

+function onAcknowledgePacket(
+  packet: Packet,
+  acknowledgement: bytes) {
+  // Receiving chain should handle this event as if the tx in packet has failed if ack bytes is not 0x0.


If the address generation works arbitrary(as commented above) the ack receiving should store the controlling interchain accounts(or send the address information to the requested entity for the account generation)

Ok, I'm trying to define interface for the host chain to handle counterparty chain's interchain account.

mossid · 2019-10-14T12:29:05Z

spec/ics-027-interchain-account/README.md

+    }
+
+    // Get handler from router
+    handler = router(msg.route())


router and type msg should be defined somewhere, however I am in favor of using router/handler here

Again, I think this is too Cosmos SDK specific. I think we should abstract what it means to "run a transaction", at least so that the Cosmos SDK specific bits can be an example implementation of the runTx function.

I agree. I will try to abstract running tx and account keeper as interface.

cwgoes

Concept ACK, a lot of the pseudocode is too SDK-specific I think, we should abstract it a bit more.

Do you think interchain multisignature accounts are out of scope of this specification or not?

cwgoes · 2019-10-14T13:22:12Z

spec/ics-027-interchain-account/README.md

+### **Desired Properties**
+
+- Permissionless
+- Fault containment: Interchain account must follow rules of its dwelling chain, even in times of byzantine behavior by the counterparty chain (the chain that manages the account)


Swap "dwelling" for "host"

cwgoes · 2019-10-14T13:22:27Z

spec/ics-027-interchain-account/README.md

+
+- Permissionless
+- Fault containment: Interchain account must follow rules of its dwelling chain, even in times of byzantine behavior by the counterparty chain (the chain that manages the account)
+- The chain(that controls the account) requesting a specific transaction from must process the results asynchronously and according to the chain's logic.


requesting a specific transaction from where or what?

cwgoes · 2019-10-14T13:23:05Z

spec/ics-027-interchain-account/README.md

+}
+```
+
+`RunTxPacketData` is used to run tx for interchain account. Tx bytes is dependent on app, and it is formed with minimal data set except data for validating signatures.


You mean without the signatures, right?

Right. I think that application logic can know what chain requests transaction and whether that chain has the right permission. So, there is no need to include a signature to the interchain account.

cwgoes · 2019-10-14T13:24:00Z

spec/ics-027-interchain-account/README.md

+type Path = string
+
+interface ModuleState {
+    addressesRegisteredChannel: Map<string, Path>


Should Address be a declared type (instead of a string)?

cwgoes · 2019-10-14T13:24:25Z

spec/ics-027-interchain-account/README.md

+}
+```
+```typescript    
+function onSendPacket(packet: Packet) {  


This function no longer exists, see the latest version of ICS 20.

Ok, I will recheck.

cwgoes · 2019-10-14T13:28:35Z

spec/ics-027-interchain-account/README.md

+
+## **Backwards Compatibility**
+
+(discussion of compatibility or lack thereof with previous standards)


If there is nothing here, just put N/A.

cwgoes · 2019-10-14T13:28:39Z

spec/ics-027-interchain-account/README.md

+
+## **Forwards Compatibility**
+
+(discussion of compatibility or lack thereof with expected future standards)


If there is nothing here, just put N/A.

cwgoes · 2019-10-14T13:28:44Z

spec/ics-027-interchain-account/README.md

+
+## **Example Implementation**
+
+(link to or description of concrete example implementation)


If there is nothing here, just put N/A.

cwgoes · 2019-10-14T13:28:51Z

spec/ics-027-interchain-account/README.md

+
+## **Other Implementations**
+
+(links to or descriptions of other implementations)


If there is nothing here, just put N/A.

cwgoes · 2019-10-14T13:29:00Z

spec/ics-027-interchain-account/README.md

+
+## **History**
+
+(changelog and notable inspirations / references)


Put a date and the first draft version.

Thunnini

I'm not sure what the interchain multisignature accounts mean. But, I guess it means that the interchain multisignature accounts are handled by multiple chains. It will be interesting feature, however, I don't know how it can be used in a real environment. I think it is better to discuss that feature in post-launch.

Thunnini · 2019-10-15T06:24:12Z

spec/ics-027-interchain-account/README.md

+### **Desired Properties**
+
+- Permissionless
+- Fault containment: Interchain account must follow rules of its dwelling chain, even in times of byzantine behavior by the counterparty chain (the chain that manages the account)


Thunnini · 2019-10-15T06:27:17Z

spec/ics-027-interchain-account/README.md

+}
+```
+
+`RunTxPacketData` is used to run tx for interchain account. Tx bytes is dependent on app, and it is formed with minimal data set except data for validating signatures.


Right. I think that application logic can know what chain requests transaction and whether that chain has the right permission. So, there is no need to include a signature to the interchain account.

Thunnini · 2019-10-15T06:28:32Z

spec/ics-027-interchain-account/README.md

+}
+```
+```typescript    
+function onSendPacket(packet: Packet) {  


Ok, I will recheck.

Thunnini · 2019-10-15T06:28:39Z

spec/ics-027-interchain-account/README.md

+  if (packet.data is RunTxPacketData) {
+    ChainAccountTx data = {}
+    // Decode transaction request
+    codec.unmarshal(packet.data.txBytes, &data)


Thunnini · 2019-10-15T06:35:26Z

spec/ics-027-interchain-account/README.md

+    // because attackers can distrupt to create an ibc managed account
+    // by sending some assets to estimated address in advance.
+    // And IBC managed account has no public key, but its sequence is 1.
+    // It can be mark for Interchain account, becuase normal account can't be sequence 1 without publish public key.


I agree. So, is it better to change the determining address login to function interface? For example, make function typetype CreateAddress = funtion(packet:Packet):Uint8Array and use it for determining address. But, I think there is no need to consider bech32, because the address is just byte address and we can interact by byte address in the codebase.

Thunnini · 2019-10-15T06:40:02Z

spec/ics-027-interchain-account/README.md

+    // It can be mark for Interchain account, becuase normal account can't be sequence 1 without publish public key.
+    account = accountKeeper.getAccount(account)
+    if (account != null) {
+      abortTransactionUnless(account.sequence === 1 && account.pubKey == null)


I think that sequence (or nonce) and a public key are very general in the blockchain. So, I think using sequence and public key is not Cosmos specific. But, in the above discussion, I agree it is better to make account keeper and account as the interface so, I think, it is good to define the account interface that has the sequence (or nonce) and public key and use those variables from the interface.

Thunnini · 2019-10-15T06:40:57Z

spec/ics-027-interchain-account/README.md

+    }
+
+    // Get handler from router
+    handler = router(msg.route())


I agree. I will try to abstract running tx and account keeper as interface.

Thunnini · 2019-10-15T06:41:27Z

spec/ics-027-interchain-account/README.md

+      ...,
+    })
+
+    // Return acknowledgement byte as 0x0 if account creation succeeds.


Thunnini · 2019-10-15T06:43:09Z

spec/ics-027-interchain-account/README.md

+function onAcknowledgePacket(
+  packet: Packet,
+  acknowledgement: bytes) {
+  // Receiving chain should handle this event as if the tx in packet has failed if ack bytes is not 0x0.


Ok, I'm trying to define interface for the host chain to handle counterparty chain's interchain account.

cwgoes · 2019-10-22T14:28:09Z

I'm not sure what the interchain multisignature accounts mean. But, I guess it means that the interchain multisignature accounts are handled by multiple chains. It will be interesting feature, however, I don't know how it can be used in a real environment. I think it is better to discuss that feature in post-launch.

Fair enough, probably better to keep the first version of this standard simple.

cwgoes · 2019-11-04T14:24:29Z

Bump. If there's anything we can do on our end to help out here let me know. I really like this proposal.

Thunnini · 2019-11-08T06:36:23Z

@cwgoes @mossid I removed the cosmos-sdk specific part in the specification and rewrote as much as possible based on a generalized interface and requirements.

cwgoes

Partway through reviewing, I will finish later today.

cwgoes · 2019-11-19T13:32:44Z

spec/ics-027-interchain-account/README.md

+modified: 2019-11-08  
+---  
+
+## **Synopsis**  


I don't think you need both the header & bold, it will format correctly in a Markdown render with just '##'. (this comment also applies to other headers in this document)

spec/ics-027-interchain-account/README.md

cwgoes · 2019-11-19T13:34:32Z

spec/ics-027-interchain-account/README.md

+
+- Permissionless  
+- Fault containment: Interchain account must follow rules of its host chain, even in times of byzantine behavior by the counterparty chain (the chain that manages the account)  
+- The chain that controls the account must process the results asynchronously and according to the chain's logic.  


Which results? Results of executing transactions?

Yes, executed transactions will return the result code such as cosmos-sdk's sdk.Result type. I think runTx should return 0x if interchain tx is succeeded, or return an error code which is not 0x if interchain tx is failed.

Sounds good to me.

(your response makes sense but can you clarify this in the document?)

spec/ics-027-interchain-account/README.md

cwgoes · 2019-11-19T13:35:07Z

spec/ics-027-interchain-account/README.md

+
+## **Technical Specification**  
+
+The implementation of interchain account is non symmetric. This means that each chain must define the way to create a interchain account for counterparty chain deterministically.


Also I'm a little confused. Does different logic have to be defined for each counterparty chain? Do you mean that the transaction bytes will be different?

It means that each chain can have a different way to deserialize transaction bytes and a different set of transactions that they can execute. For example, chains that use cosmos-sdk will deserialize tx bytes by amino, but if counterparty chain is a smart contract on Ethereum, it may deserialize tx bytes by ABI that is minimal serialize algorithm for smart contract. And a smart contract on Ethereum can't execute the staking, governance transaction if it doesn't support such feature. So, I think interchain account defines the general way to register interchain account and transfer tx bytes, and counterparty chain is responsible to deserialize and execute the tx bytes, and sending chain should know how counterparty chain handle tx bytes in advance.

Makes sense, just include the above paragraph in the spec so that it's clear.

spec/ics-027-interchain-account/README.md

cwgoes · 2019-11-19T13:35:52Z

spec/ics-027-interchain-account/README.md

+The chain must reject the transaction and must not make a state transition in the following cases:
+
+- The IBC transaction fails to be deserialized.
+


(remove empty line)

spec/ics-027-interchain-account/README.md

cwgoes

See comments. Generally looking pretty good.

spec/ics-027-interchain-account/README.md

cwgoes · 2019-11-19T14:55:48Z

spec/ics-027-interchain-account/README.md

+
+## **Example Implementation**  
+
+Pseudocode for cosmos-sdk: https://github.com/everett-protocol/everett-hackathon/tree/master/x/interchain-account


It would also be helpful to reason about how this would work on Ethereum, which has a sufficiently programmable system that it should be able to implement this spec.

I think that it can be implemented in a smart contract level with using CALL, DELEGATECALL, CALLCODE opcode that are used to delegate call to some function in callee. I will research more and try to implement pseudocode in Solidity.

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

Thunnini · 2019-12-02T07:49:01Z

I fixed the misspelling and added some more specific description.
And, I added the rough way to implement the interchain account on Ethereum's smart contract. See the link for more details. https://github.com/everett-protocol/ethereum-interchain-account
My implementation of interchain account on Ethereum is a very early stage of POC. It just implements the minimal set of the interface needed to implement interchain account specification because we don't have the implementation of IBC on Ethereum yet. But it indicates that the interchain account can operate on the Ethereum smart contract level.

Thunnini · 2019-12-02T07:57:32Z

@cwgoes @mossid

cwgoes

Almost there, a few final requested changes. cc @mossid, two reviews would be prudent.

cwgoes · 2019-12-02T11:15:11Z

spec/ics-027-interchain-account/README.md

+
+- Permissionless  
+- Fault containment: Interchain account must follow rules of its host chain, even in times of byzantine behavior by the counterparty chain (the chain that manages the account)  
+- The chain that controls the account must process the results asynchronously and according to the chain's logic.  


(your response makes sense but can you clarify this in the document?)

cwgoes · 2019-12-02T11:15:25Z

spec/ics-027-interchain-account/README.md

+- Permissionless  
+- Fault containment: Interchain account must follow rules of its host chain, even in times of Byzantine behavior by the counterparty chain (the chain that manages the account)  
+- The chain that controls the account must process the results asynchronously and according to the chain's logic.  
+- Sending and receiving transactions will be processed in an ordered manner.  


As in, first in first out? Be specific (there could be many kinds of orderings).

spec/ics-027-interchain-account/README.md

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

… the counterparty chain

Thunnini · 2019-12-02T13:22:09Z

@cwgoes Thank you for your advice and for fixing the grammar and misspelling. I fixed it. Please check again.

cwgoes · 2019-12-02T14:40:00Z

@cwgoes Thank you for your advice and for fixing the grammar and misspelling. I fixed it. Please check again.

I think there are still a few outstanding comments, please check.

(I would fix them, but I can't commit to your branch)

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

Thunnini · 2019-12-02T14:45:21Z

@cwgoes I handled missed comments. Do I have anything left?

mossid

LGTM, commented minor changes

spec/ics-027-interchain-account/README.md

Co-Authored-By: Joon <torecursedivine@gmail.com>

spec/ics-027-interchain-account/README.md

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

cwgoes

ACK (as draft). Thanks & nice job 🎉 !

mossid

Thanks for your contribution, great job! 💯

Add interchain account initial draft spec

829d1a1

mossid suggested changes Oct 14, 2019

View reviewed changes

cwgoes reviewed Oct 14, 2019

View reviewed changes

Thunnini commented Oct 15, 2019

View reviewed changes

cwgoes mentioned this pull request Oct 17, 2019

ICS 13: Interchain Address Format #22

Closed

Major revision

a502088

Thunnini force-pushed the Thunnini/ics-027-interchain-account branch from 7da7631 to a502088 Compare November 8, 2019 06:17

cwgoes suggested changes Nov 19, 2019

View reviewed changes

Thunnini force-pushed the Thunnini/ics-027-interchain-account branch 2 times, most recently from ed1e77e to 8db2fc5 Compare December 2, 2019 06:48

Fix misspelling

654882e

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

Thunnini force-pushed the Thunnini/ics-027-interchain-account branch from 8db2fc5 to 654882e Compare December 2, 2019 06:51

Thunnini added 6 commits December 2, 2019 16:14

Change the words trustless in motivation to more specific

7f51504

Make onTxFailed be able to get acknowledgement

35f75d3

Add more description to the way generates an address

dd40c71

Add more description about non-symmetric

637f094

Add interchain account on Ethereum link to example

c3ed630

Add history

034601d

cwgoes suggested changes Dec 2, 2019

View reviewed changes

Fix grammar

12d797e

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

Thunnini force-pushed the Thunnini/ics-027-interchain-account branch from 0238a3c to 12d797e Compare December 2, 2019 12:42

Thunnini added 2 commits December 2, 2019 22:03

Add some more detailed description about ordered manner

a5bde23

Add description about how to distinguish signers that was not made by…

341d9f1

… the counterparty chain

Thunnini and others added 2 commits December 2, 2019 23:40

Add requires, kind & modify date

117efb4

Fix grammer

06d9359

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

Thunnini force-pushed the Thunnini/ics-027-interchain-account branch from 4e58084 to 06d9359 Compare December 2, 2019 14:43

mossid reviewed Dec 2, 2019

View reviewed changes

Thunnini and others added 3 commits December 3, 2019 00:50

Fix out of date link

153dd98

Co-Authored-By: Joon <torecursedivine@gmail.com>

Fix typo

4066987

Co-Authored-By: Joon <torecursedivine@gmail.com>

Add simple definition for Tx

a702768

cwgoes reviewed Dec 2, 2019

View reviewed changes

spec/ics-027-interchain-account/README.md Outdated Show resolved Hide resolved

Add more detailed description about the case tx should be rejected

8b7e424

Co-Authored-By: Christopher Goes <cwgoes@pluranimity.org>

cwgoes approved these changes Dec 2, 2019

View reviewed changes

mossid approved these changes Dec 2, 2019

View reviewed changes

cwgoes merged commit a95b4fd into cosmos:master Dec 2, 2019

Thunnini deleted the Thunnini/ics-027-interchain-account branch December 3, 2019 08:48


		## Backwards Compatibility

		(discussion of compatibility or lack thereof with previous standards)


		## Forwards Compatibility

		(discussion of compatibility or lack thereof with expected future standards)


		## Example Implementation

		(link to or description of concrete example implementation)


		## Other Implementations

		(links to or descriptions of other implementations)


		## History

		(changelog and notable inspirations / references)


		## Technical Specification

		The implementation of interchain account is non symmetric. This means that each chain must define the way to create a interchain account for counterparty chain deterministically.

		The chain must reject the transaction and must not make a state transition in the following cases:

		- The IBC transaction fails to be deserialized.


		## Example Implementation

		Pseudocode for cosmos-sdk: https://github.com/everett-protocol/everett-hackathon/tree/master/x/interchain-account

ICS 027: Interchain account #278

ICS 027: Interchain account #278

Conversation

Thunnini commented Sep 26, 2019

Changes

mossid left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thunnini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwgoes commented Oct 22, 2019

cwgoes commented Nov 4, 2019

Thunnini commented Nov 8, 2019

cwgoes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thunnini commented Dec 2, 2019 • edited Loading

Thunnini commented Dec 2, 2019

cwgoes left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thunnini commented Dec 2, 2019

cwgoes commented Dec 2, 2019 • edited Loading

Thunnini commented Dec 2, 2019

mossid left a comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

mossid left a comment • edited Loading

mossid left a comment •

edited

Loading

Thunnini commented Dec 2, 2019 •

edited

Loading

cwgoes left a comment •

edited

Loading

cwgoes commented Dec 2, 2019 •

edited

Loading

mossid left a comment •

edited

Loading