Technical Debt Refactoring

[aaronc]

Copying my notes from today's architecture call here:

Broader Framing

• There's a lot of technical debt in the SDK. This technical debt makes:
  • bugs and performance issues more likely
  • building on top of the SDK more challenging
  • means a refactor is inevitable that will affect users
• Broad areas of focus:
  • types/ is crowded and lacks coherence, too many globals, some inefficient and/or buggy code
  • x/auth suffers from conflation of authentication with vesting and legacy of tracking balances
  • x/bank suffers from legacy of balances stored in x/auth and supply stored in x/supply, plus a general focus on a single denom
  • store is being addressed separately, but in general suffers from performance issues related to IAVL and the non-atomicity of the multi-store
  • simulations depend on simapp, all modules use simulations, simapp depends on all modules, the whole design circular

General strategy

• redesign types/ from the ground up
• separate vesting from x/auth
• audit x/bank and design it better for multiple denoms
• remove simapp ↔ simulations ↔ module dependencies

Separate go modules

Consider splitting the SDK into multiple go modules (in this same mono-repo) for a) a more disciplined dependency graph and b) decoupled release cycles. High level modules would be:

• cosmovisor (example git tag: cosmovisor/v1.0)
• x/* modules (example git tag: x/bank/v0.2)
• base/ (see below): everything needed by every module developer to build modules
• maybe rosetta
• etc.

Pros

• decoupled release cycles could allow sub-teams to work more independently
• enforces better dependency graph
• can allow some modules to reach 1.0 sooner (i.e base, then x/auth, then x/staking, etc.)
• separate versioning is clearer and allows apps to update specific modules more independently

Cons

• release cycles could be more complex (if we don't figure out proper processes &/or automation)
• there could be some complexities in linking between modules in the repo
• more complex testing and CI config - need to have a separate test command per module, possibly more parallelizable though
• lots of work refactoring modules

types/ Summary Audit and Recommendations

A more detailed audit will be posted later. This doesn't cover sub-packages of types/ yet like types/module.

• consider creating a core/ package to replace types/ which modules can migrate to one by one, gradually
• core/ could become its own go module
• what core/ would include from the root types/ package and what would change:
  • keep MsgRequest and make MsgRequest.GetSigners() return []string and deal with bech32 at the framework layer - this also opens the door for other address representations like DIDs, etc.
  • in a separate core/address package provide these for when they are needed:
    • ParseAddress(sdk.Context, string) (AccAddress, error)
    • AccAddress.ToString(sdk.Context) string
  • keep Coin, convert Coins to use a tree structure under the hood, ditto for DecCoin
  • either keep Int, Uint, and Dec, or move to a types/math package. Dec needs to be refactored and fixed
  • Context should probably be converted to a pure interface with implementations elsewhere, ditto for StoreKey types, etc.
  • most other things from the root types/ package would not be included
• base would also include some sub-packages types/ like types/module (upgraded for ADR 033), codec, store APIs, testing infrastructure, simulations, maybe baseapp, etc.
• proof of concept branch: https://github.com/cosmos/cosmos-sdk/compare/aaronc/base-poc

[alessio]

Thanks for kicking off this conversation Aaron. There are a bunch of very good ideas in your proposal. The first comment that comes to mind is: what is the benefit of creating a package base? Can't we just promote all packages in types that are not types and should not be there to top level package? Examples: types/address, types/errors, types/kv, types/grpc [1].
IMHO it had better go for a flatter structure than adding another directory level.

(more comments will follow in the coming days)

[1] The package types/grpc is particularly odd as it comprises only const declaration :-/

[amaury1093]

Yes please! I also suggest we could try to follow the same folder arch in proto/cosmos/core/* and in the core/* go module.

[aaronc]

What do you think @alessio ?

[alessio]

sure, not a problem

[aaronc]

updated the examples to use core instead of base

[robert-zaremba]

I also prefer core. The main idea is to break down the content of current types into sub package - as we started with types/address.

[aaronc]

As a follow-up to our last discussion around this, I created this issue in regen-ledger for us to do a trial run of this approach: regen-network/regen-ledger#319. Will follow-up when we have any findings to report.

[robert-zaremba]

I think the biggest mess is with x/auth. I like the idea of deprecating it, and creating x/authn with bunch of new packages is core for things like: Addressable, Credentials, general solutions for middlewares / antehandlers....
8 months ago I started refactoring of x/auth and abandoned it because it was endless - too many complex dependencies, and hacks to avoid import cycle errors.

[aaronc]

Yes, everything related to authentication is a big source of technical debt

[aaronc]

Replaced by #10582