cosmos-sdk icon indicating copy to clipboard operation
cosmos-sdk copied to clipboard

EPIC: Separate all SDK modules into standalone go modules

Open aaronc opened this issue 3 years ago • 3 comments

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 .

  • [ ] Update the ADR for reflecting the implementation (first draft: https://github.com/cosmos/cosmos-sdk/pull/11873)

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.

  • [x] Single module (x/nft) tests using app.yaml instead of simapp (full feature branch in #11900), being broken down into separate PRs:
    • [x] #11912
    • [x] #11913
    • [x] #11914
    • [x] #11915
    • [x] #11924
    • [x] additional app wiring setups from #11900
      • [x] #12019
      • [x] #12032
      • [x] #12036
      • [x] #12035
      • [x] #12038
    • [x] x/nft cli tests and testutil/network from #11900
  • [x] #11904
  • [x] #11905
  • [x] #11906
  • [x] #11917
  • [x] #11925
  • [x] #11935
  • [x] #11937

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:

  • create a protobuf Module config message and register a module implementation with appmodule.Register
  • inject the new module into simapp
  • ~migrate CLI tests to use appconfig~ Superseded by #12398
  • ~migrate simulations and any other code requiring simapp~ Superseded by #12398

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

  • [x] #12023
  • [x] #12024
  • [x] #12084
  • [x] #12073
  • [x] #12194
  • [x] #12069
  • [x] #12298
  • [x] #12238
  • [x] #12302
  • [x] #12083
  • [x] #12036
  • [x] #12195
  • [x] #12292
  • [x] #12234
  • [x] x/genutil
  • [x] #12274
  • [x] #12300

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

  • [ ] #12054
  • [ ] #12055
  • [x] #12056
  • [x] #12068
  • [ ] depinject module upgrades based to make this work easier (nice to have):
    • [x] #11907
    • [x] #11908
    • [ ] #11909
    • [x] #11910
    • [x] #11911
    • [x] #11943
    • [x] #11944
    • [ ] #12252
  • [x] #11903
  • [x] #12237
  • [x] #12400

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:

  • [x] #12535
  • [x] #12544
  • [x] #12528
  • [x] #12612
  • [x] #12619
  • [x] #12546
  • [x] #12584

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:

  • [ ] backport all commits from the above work that affect the root github.com/cosmos/cosmos-sdk go module to a release branch (probably release/v0.46.x after the official v0.46.x release is tagged), commits that affect sub-modules like depinject do not need to be backported
  • [ ] 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
  • [ ] backport and tag both the standalone simapp and sdk without simapp

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.

  • [ ] x/nft
  • [ ] x/params
  • [ ] x/group
  • [ ] x/authz
  • [ ] x/feegrant
  • [ ] x/crisis
  • [ ] x/capability
  • [ ] x/upgrade
  • [ ] x/evidence
  • [ ] x/auth
  • [ ] x/bank
  • [ ] x/staking
  • [ ] x/slashing
  • [ ] x/distribution
  • [ ] x/mint
  • [ ] x/genutil
  • [ ] runtime (this will need to be carved out last)

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 avatar May 09 '22 01:05 aaronc

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

julienrbrt avatar Jun 29 '22 13:06 julienrbrt

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

aaronc avatar Jun 29 '22 14:06 aaronc

backport all commits from the above work that affect the root github.com/cosmos/cosmos-sdk go module to a release branch (probably release/v0.46.x after the official v0.46.x release is tagged), commits that affect sub-modules like depinject do not need to be backported

Is there a reason we have to backport like this?

tac0turtle avatar Jul 26 '22 08:07 tac0turtle