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

Restructure Interchain Security repo for release #77

Closed
mpoke opened this issue Apr 29, 2022 · 10 comments
Closed

Restructure Interchain Security repo for release #77

mpoke opened this issue Apr 29, 2022 · 10 comments

Comments

@mpoke
Copy link
Contributor

mpoke commented Apr 29, 2022

No description provided.

@jtremback
Copy link
Contributor

Currently, we are shipping a repo with one blockchain that can be both a provider and a consumer chain. This is mainly due to the fact that our repo is loosely based on the structure of the IBC transfer module which is designed to work between two identical chains (identical at least as far as the IBC transfer module is concerned).

We need to design a better repo for several reasons.

First, there are modules which, when enabled, crash a consumer chain. One of them is Distribution. It tries to assign rewards to a validator it doesn't recognize from the Staking module (because on a consumer chain there are no validators in the staking module), which results in the chain crashing. Slashing may also be an issue, but I'm not completely sure (@sainoe?).

Second, we want to make it easy for consumer chains, built outside of our repo to be tested. The confusing hacks we use to make a single codebase work as both as consumer and provider chain will not be easy for downstream developers to follow and work with. It would be best if there was a dedicated consumer chain codebase as an example (see #59).

@mpoke
Copy link
Contributor Author

mpoke commented Apr 30, 2022

@AdityaSripal Do you have any insights in what's the best way to approach this issue?

@okwme
Copy link
Contributor

okwme commented May 4, 2022

seems like we should have interchain-security-provider and interchain-security-consumer repos, right?

@mpoke
Copy link
Contributor Author

mpoke commented May 4, 2022

seems like we should have interchain-security-provider and interchain-security-consumer repos, right?

Yeah, that would be one option. The problem is that there is some code common to both type of chains, which would need to be duplicated (e.g., the proto definitions of the CCV packets).

Another alternative would be to keep one single repo, similar to the existing structure, and create two app.go files, one for each chain. Basically, we create to different binaries. @danwt is currently working on this (#84).

@mpoke
Copy link
Contributor Author

mpoke commented May 4, 2022

I'm not sure what's the best approach here, since I'm not sure what's the intended target.

  • Interchain Security is an IBC application. @AdityaSripal would it eventually be part of ibc-go?
  • Would the provider CCV module (together with all the Cosmos SDK changes) be part of the cosmos-sdk repo? Or is it possible to import the provider module on a need-only basis (e.g., in https://github.com/cosmos/gaia)?

@mpoke mpoke moved this from Next to In Progress in Replicated Security May 4, 2022
@mpoke mpoke moved this from In Progress to Next in Replicated Security May 4, 2022
@okwme
Copy link
Contributor

okwme commented May 5, 2022

one important item is that we may want/need to have the consumer chain support multiple versions of the SDK. Would a separate repo make this more viable?

@mpoke
Copy link
Contributor Author

mpoke commented May 5, 2022

@jtremback was suggesting that it may be a good idea to support multiple versions for the provider as well (see #63). This will enable chains that do not upgrade to the SDK 0.46 to still use Interchain Security and be a provider chain. I don't think it's more work, since we already support SDK 0.45 for both consumer and provider.

@AdityaSripal
Copy link
Member

Interchain security won't go into ibc-go because it isn't managed by the ibc-go team. I think it needs to be in a separate repo because it is a large complex application.

Generally, if an application requires its own independent team to manage, they should have their own repo to manage deadlines, priorities, release schedules etc.

@mpoke
Copy link
Contributor Author

mpoke commented May 5, 2022

Duplicate expected_keeper.go (one for each type of chain).

@danwt danwt self-assigned this May 5, 2022
@danwt
Copy link
Contributor

danwt commented May 5, 2022

As per the feedback of the meeting it makes sense to have the provider be a stripped version of gaia, or just a basic gaia app, and for the consumer to be minimal.

The consumer could and should ultimately be extended with democracy and cosmwasm, but that will be a later step.

@danwt danwt removed their assignment May 9, 2022
@mpoke mpoke added product and removed ccv-core labels May 10, 2022
@mpoke mpoke moved this from Next to In Progress in Replicated Security May 10, 2022
@mpoke mpoke removed the ccv-core label Jun 8, 2022
@mpoke mpoke closed this as completed Jun 8, 2022
Repository owner moved this from In Progress to Done in Replicated Security Jun 8, 2022
ThanhNhann pushed a commit to decentrio/interchain-security that referenced this issue Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants