Cosmos-sdk: move transfer outside x/ibc as a standalone module

[colin-axner]

Summary

Reorganize the x/ibc module into a logical file structure.

Problem Definition

Jumping into ibc at the moment is confusing and difficult at the moment and understandably so since it is undergoing heavy development. There is a lot of layering going on almost like the x/ibc is attempting to be both a module and a SDK. The top level houses folders to low level infrastructure (02-client, 03-connection, 04-channel, 05-port), and client implementations (07-tendermint, 09-localhost), and application level interactions (20-transfer). It also houses all the files necessary for ibc to function as a module.

This is just the current structure. I assume there are quite a few more client and application level implementations on the way.

Proposal

I'm not sure what the ideal layout would look like, but I felt it is a good time to start up the conversation. I think the SDK greatly benefited from having a standardized module structure and controversial naming issues #6024 never go away. But coming up with a deliberate scheme from the beginning will save time on arguments down the line.

I think it's useful to separate low level stuff from higher level application level implementations. Many developers will never need to look at channel and client but they will probably look into transfer and tendermint.

Maybe the current structure could be supplemented by sufficient documentation. While the ICS specs are useful for implementation purposes, they are too abstract for introduction and conceptual purposes and therefore there is a lack of documentation on the purposes and interconnection of the packages on this codebase.

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[fedekunze]

I understand the concern. The main benefit of the current structure is to keep ICS implementations separated in their submodules so that it eases auditors ' work in comparing the implementation against the spec.

That's primarily why I added a x/ibc/spec/ folder with the "SDK spec" of the ibc module and relevant work. It's far for completed and if you want to take a stab at that feel free to work on it.

I do support moving ICS20 (currently in x/ibc/20-transfer/) to a separate package (x/transfer) since it's technically an independent module. This way the ibc module will only have IBC-TAO (transport authentication and ordering) logic.

We can additionally create a README on the IBC module because I think users are not actually looking into the spec contents.

cc: @cwgoes thoughts?

[cwgoes]

I do support moving ICS20 (currently in x/ibc/20-transfer/) to a separate package (x/transfer) since it's technically an independent module. This way the ibc module will only have IBC-TAO (transport authentication and ordering) logic.

I am in favour of this. It will be clearer for application developers as well.

I don't think the x/ibc submodules need to match the ICS specs exactly, but the ICS specs are designed to capture the right sort of abstraction layering, so I'm not sure there is a simpler option - open to concrete proposals though.

[fedekunze]

updated title

[marbar3778]

I do support moving ICS20 (currently in x/ibc/20-transfer/) to a separate package (x/transfer) since it's technically an independent module.

If we have x/bank & x/transfer then it is unclear to a new user what they should use, at first glance. If the transfer module has the bank module as a dependency (which it seems to) I think it should live within the bank module as a submodule. x/bank/transfer or something more worded toward ibc x/bank/ibc-transfer

[cwgoes]

If we have x/bank & x/transfer then it is unclear to a new user what they should use, at first glance. If the transfer module has the bank module as a dependency (which it seems to) I think it should live within the bank module as a submodule. x/bank/transfer or something more worded toward ibc x/bank/ibc-transfer

Hmm - I think there should be one module (x/bank) that handles the actual logic, and another (x/transfer or x/bank/transfer) that handles sending/receiving of IBC packets and calling the x/bank handler appropriately. It can be a submodule of x/bank, but it shouldn't be enabled by default - only if the user specifically elects to enable it. This pattern (core module with state transition logic, and a "bridge module" to deal with sending/receiving packets) will be very common, we should come up with a standard way of doing it.

[marbar3778]

yes I agree with you that it should not be enabled by default. If you want it enabled then you will have to bring in bank and explicitly call transfer the same way it is now, so this builds the mental model of things working together.

At least how I am seeing something like x/transfer is that there is the possibility of it working on its own (without bank).

[fedekunze]

x/bank/transfer or something more worded toward ibcx/bank/ibc-transfer

No more submodules plz. The only reason why we have them is to encapsulate each ICS and ease audit against the spec.

[cwgoes]

At least how I am seeing something like x/transfer is that there is the possibility of it working on its own (without bank).

How would that work (what would it do other than call the x/bank keeper)?

[marbar3778]

ah i meant if i were a new comer and didnt understand that x/transfer has a dep on bank i would think i can use it standalone. But in the PR bez suggested x/ibc-transfer which is good imo