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

Move x/ibc to it's own repository #8501

Closed
4 tasks
colin-axner opened this issue Feb 3, 2021 · 11 comments · Fixed by #8735
Closed
4 tasks

Move x/ibc to it's own repository #8501

colin-axner opened this issue Feb 3, 2021 · 11 comments · Fixed by #8735
Assignees
Milestone

Comments

@colin-axner
Copy link
Contributor

Summary

IBC is very complicated and has it's own versioning scheme. To reduce version collisions with the SDK, developer overhead, and noise, we think it is best to move x/ibc to its own repository. This allows for more flexibility on the IBC side to cater to our application developers.

This issue is meant to signal to any relevant parties that this migration will be occurring soon. The new repository for ibc core will be cosmos/ibc-go. We will likely create another repository for applications module cosmos/ibc-app

We are open to arguments for why this is the wrong approach, however we have gotten general consensus from the IBC ecosystem that this is desirable.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@tac0turtle
Copy link
Member

What is the reasoning for an applications' module repo?.

I would advise to keep the application modules and core IBC together. Dependency management will become hard by needing to look across two repo to find the needed changes required for updates.

Generally in favor of IBC having its own repo, have you also weighed the approach of having an additional go.mod in the cosmos-sdk repo?

@colin-axner
Copy link
Contributor Author

colin-axner commented Feb 3, 2021

What is the reasoning for an applications' module repo?.

I think logical separation, but I'm not entirely sure the exact benefits. @cwgoes ?

Dependency management will become hard by needing to look across two repo to find the needed changes required for updates.

Happy you mentioned this, because these sort of issues that are my concern being a relatively inexperienced engineer on dependency management. Would this be difficult for the IBC application modules or for an end user like gaia?

have you also weighed the approach of having an additional go.mod in the cosmos-sdk repo?

The primary motivator, at least from my point of view, for separating IBC core from the SDK is the versioning scheme. After IBC 1.0 was cut, there were small changes that were not IBC breaking, but that were breaking changes for the SDK versioning scheme. These changes were ideal for application developers, but would have to wait on the SDK's release cycle. If IBC had been it's own repo, we could have cut a minor non-breaking release for the changes. I suspect this, non-IBC-breaking, but SDK breaking, changes will occur frequently.

Furthermore, when IBC 1.1 begins development, there could be awkward release points or development workflows, where IBC 1.1 is under heavy construction, but SDK wants to do a new release. It would be troublesome to force IBC 1.1 to be developed on a separate branch and it would be awkward to cut a release when IBC is transitioning from 1.0 to 1.1.

In addition to the above concerns, issues generally tend to get drowned out by the SDK issues for me. It's hard to keep track of IBC issues unless they are labeled. IBC has generally been developed in its own isolated space on the SDK, and it seems like it could be beneficial to make this space explicitly separate.

That said, another concern I have is tooling. Do you think it would be difficult to maintain tooling generated by the SDK and reused in the IBC repo?

@cwgoes
Copy link
Contributor

cwgoes commented Feb 3, 2021

I think logical separation, but I'm not entirely sure the exact benefits. @cwgoes ?

Logical separation matching the new specification format, and it will match the way ibc-rs is set up I believe.

@tac0turtle
Copy link
Member

The dependency hell will start when a user wants to use an application module that uses sdk v0.43, but their app uses ibc 1.1 and sdk 1.0. It can be managed, but will be difficult for developers to keep in their mind.

Doing two repos is a lot easier than three, IMO.

Not sure how ibc-rs handles application modules vs core IBC but would advocate for them to use a single repo. In rust a single repo that contains all of it is easier to handle due to cargo, but since go uses github tags for versions it gets complicated

@tac0turtle
Copy link
Member

Not sure your timeline here, but I think this should not be done till after there has been more adoption or a breaking change in IBC. Having the IBC 1.0 in two places (ibc-go repo, cosmos-sdk 0.41 & 0.42) will cause lots of confusion.

@colin-axner
Copy link
Contributor Author

Not sure your timeline here, but I think this should not be done till after there has been more adoption or a breaking change in IBC. Having the IBC 1.0 in two places (ibc-go repo, cosmos-sdk 0.41 & 0.42) will cause lots of confusion.

We are flexible on the timeline given there is no active development of IBC. That being said, it's also an ideal time since there is no active development being done, less developer disruption. I agree moving ibc will cause lots of confusion, but this seems unavoidable given that SDK development isn't going to slow down anytime soon.

I propose, we wait at least until v0.42 is cut (since it is imminent). We make announcement on all SDK/IBC comms that IBC will be migrated after SDK version X is cut (X could be v0.42.0, open to opinions). We update the issue/feature templates on the SDK to indicate IBC issues should be opened in the ibc repo, we update the x/ README to specify that IBC has been moved and is located in a separate repo.

The ICS repo is being migrated within the next week to a cosmos/ibc repo, it seems like good timing to rebrand both the spec repo and the ibc-go repo (there could even be a blog post). My biggest concern would be disrupting any implementations reliant on the IBC code (wasm cc @ethanfrey any concerns?, ibc-rs maybe?). The proto files have already been decided to be moved to the spec repo, so maybe we could start with that migration (since I think that is all ibc-rs uses).

The other concern is keeping up with the SDK release cycle. I think the ibc-go repo needs to have a commitment to always backporting the latest SDK update to the latest IBC release. I'm willing to commitment to this. But it'd be best to be able to migrate IBC, do a release of the migrated code (since import paths will change) and then start the commitment. So at least a one month period between SDK releases would be ideal.

After writing all this, I think it'd be best to make a call and sync up live with anyone who has concerns about the migration and to strategically plan all the steps necessary to execute it without disruption

@colin-axner
Copy link
Contributor Author

The dependency hell will start when a user wants to use an application module that uses sdk v0.43, but their app uses ibc 1.1 and sdk 1.0. It can be managed, but will be difficult for developers to keep in their mind.

Also to reply to this comment, I believe the idea is that this dependency hell management is unavoidable given where the space is going (zones/hubs will be using modules dependent on the SDK but housed outside the SDK, thanks to help from Atlas). However, if the primary benefits of separating ibc-go and ibc-app is logical separation. I'd prefer to use one repo and different folders (like we do now) to manage this.

The benefits would be less release management and more active maintenance on ibc-app modules. I fear ibc-app would turn into a graveyard without an active team. The benefit of including app modules in ibc-go is a promise by the core ibc team to maintain those app modules with the latest SDK versions (and possibly the latest ICS versions)

The logical separation between ibc-go and ibc-app is no where near as significant as the logical separation from cosmos-sdk and ibc-go

@adizere
Copy link
Contributor

adizere commented Feb 8, 2021

Not sure how ibc-rs handles application modules vs core IBC but would advocate for them to use a single repo. In rust a single repo that contains all of it is easier to handle due to cargo, but since go uses github tags for versions it gets complicated

We have a single "repo" (called a crate in Rust, yep managed by cargo) which contains both the ibc core and the application. I'm not aware of any plan to separate the dev/releasing of apps from that of core modules.

I guess keeping the apps/ and core/ in separate folders (even if in the same repo & versioning and release cycle) will still achieve a level of separation that resembles the new IBC specification format.

My biggest concern would be disrupting any implementations reliant on the IBC code (wasm cc @ethanfrey any concerns?, ibc-rs maybe?)

I don't think so.

@AdityaSripal
Copy link
Member

The only reason I can see for having app modules in a separate repo is to integrate them with the rest of the ecosystem.

I was imagining that the ibc app modules might be housed and maintained in cosmos/modules and registered on Atlas alongside app modules written by third parties. Thus, the app modules IG writes is fully integrated into the same ecosystem as third-party app developers and doesn't have some privileged position. Although, it would still benefit from the fact that IG is a reputable maintainer.

@cwgoes
Copy link
Contributor

cwgoes commented Feb 9, 2021

I guess keeping the apps/ and core/ in separate folders (even if in the same repo & versioning and release cycle) will still achieve a level of separation that resembles the new IBC specification format.

I think this is fine, at least for the time being as a first step.

The only reason I can see for having app modules in a separate repo is to integrate them with the rest of the ecosystem.

This is true, but we gain the main benefits of "behaving like the rest of the ecosystem" just by separating from the SDK - and we can separate out application modules in the future, that option remains. I guess my other question is whether other modules will be separated from the SDK - and if the ics-20 module should live where those might live - but it sounds like this is not the current trajectory of Regen's work.

Let's also synchronise this migration with the specification migration (thanks @colin-axner for the suggestion).

This was referenced Feb 23, 2021
@colin-axner
Copy link
Contributor Author

colin-axner commented Mar 1, 2021

PRs to merge before we can remove IBC and move it to ibc-go:

@amaury1093 amaury1093 added this to the v0.42 milestone Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants