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

EPIC: Separate all SDK modules into standalone go modules #11899

Closed
78 of 81 tasks
aaronc opened this issue May 9, 2022 · 4 comments
Closed
78 of 81 tasks

EPIC: Separate all SDK modules into standalone go modules #11899

aaronc opened this issue May 9, 2022 · 4 comments
Assignees
Labels
T:Epic Epics

Comments

@aaronc
Copy link
Member

aaronc commented May 9, 2022

Overview

This issue outlines a roadmap for breaking up all of the x/... SDK modules and simapp into standalone go modules using the app wiring design .

Problem Definition

The key culprit of the SDK not being decomposable into separate go modules is the dependency on simapp. simapp imports every module but every module and most packages also import simapp into their tests. Overall there are 184 references to simapp in other packages (run Code > Analyze Code > Backwards Dependencies… on simapp in Goland or IntelliJ to create this report). The reason for such widespread usage of simapp in tests is because there needs to be a way to do integration tests in the SDK and simapp provides the key test fixture. Unfortunately, this creates a Gordian knot where all the modules depend on each other and breaking them up is almost impossible. App wiring provides a solution to this problem by creating a way to quickly spin up integration test fixtures locally in each module rather than using a single SDK wide test fixture that brings all these problems.

Phase 1: App Wiring MVP

In this first phase, we create an MVP of app wiring that has these requirements:

  • can refactor a single module (we’re choosing x/nft) to not depend on simapp at all for testing or any other reason
  • can refactor part of simapp/app.go to use the new app wiring while still wiring up everything else as before - this is required to make the app wiring migration gradual and opt-in

For now, the app wiring framework that we will create will forego dealing with any of the messy issues related to protobuf generated code and semantic versioning. This means basically that app wiring will use the global protobuf registry for decoding the app config and the existing gogo proto-based codec package will be used elsewhere.

Phase 2: Decouple modules and packages from simapp

Following the patterns demonstrated in x/nft do the same for each module and package which depends on simapp in the SDK, essentially:

Modules should be migrated starting with those with fewer dependencies on other modules first.

These other issues can also be addressed incrementally during phase 2:

As this work proceeds, simapp itself can be refactored to use its app.yaml more and more - this can be done incrementally as each module is migrated or at the end when simapp is standalone.

Other packages besides the x/* modules depend on simapp as well, usually to call simapp.MakeTestEncodingConfig() or simapp.DefaultConsensusParams(). DefaultConsensusParams should have been moved in Phase 1, but MakeTestEncodingConfig may require app wiring because it used to build a codec with some modules registered. A strategy for decoupling the following packages from simapp will need to be created:

At this point, a review of all the migrated modules should be performed. The review should verify the tests coverage and the test behaviors is kept unchanged compared to before the migration.

Phase 3: Make simapp a standalone go module

Instructions in https://github.com/golang/go/wiki/Modules#is-it-possible-to-add-a-module-to-a-multi-module-repository must be followed. Also, we want to make tags off of a release branch, not main. It is expected that these steps will be required:

  • make simapp a standalone go module github.com/cosmos/cosmos-sdk/simapp which imports github.com/cosmos/cosmos-sdk probably using a replace tag to start. Any remaining dependencies of the rest of the SDK on simapp must be removed by now

Phase 4: Make all modules standalone

Again following the instructions from https://github.com/golang/go/wiki/Modules#is-it-possible-to-add-a-module-to-a-multi-module-repository and starting with the modules with the fewest dependencies to the most.
List of To-dos when creating a go.mod: https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#gomod

Phase 5

NOTE: it may not be possible to move all modules out of the core go module with this strategy alone because of other cyclic dependencies between each other (such as x/auth and x/bank). Other strategies may be needed or there may be an inseparable set of modules that stay in the SDK for now and are decoupled later if needed.

@aaronc aaronc added T:Epic Epics and removed meta-issue labels May 9, 2022
@tac0turtle tac0turtle changed the title Separate all SDK modules into standalone go modules EPIC: Separate all SDK modules into standalone go modules May 9, 2022
@aaronc aaronc pinned this issue May 10, 2022
@aaronc aaronc self-assigned this May 10, 2022
@kocubinski kocubinski self-assigned this May 24, 2022
mergify bot pushed a commit that referenced this issue Jun 8, 2022
## Description

ref: #12084, #11899



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
@julienrbrt julienrbrt self-assigned this Jun 16, 2022
@cosmos cosmos deleted a comment Jun 27, 2022
@cosmos cosmos deleted a comment Jun 27, 2022
@julienrbrt
Copy link
Member

julienrbrt commented Jun 29, 2022

The work on this EPIC should use the branch epic/app-wiring as base.

@julienrbrt julienrbrt mentioned this issue Jun 29, 2022
19 tasks
@aaronc
Copy link
Member Author

aaronc commented Jun 29, 2022

We've never used feature branches and have no process for them. Let's just consider this epic paused

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:Epic Epics
Projects
Status: 🥳 Done
Development

No branches or pull requests

6 participants