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 codebase for a module-based separation of concerns #2111

Closed
8 tasks
Tracked by #2529
cwgoes opened this issue Nov 7, 2023 · 2 comments · Fixed by #3670
Closed
8 tasks
Tracked by #2529

Restructure codebase for a module-based separation of concerns #2111

cwgoes opened this issue Nov 7, 2023 · 2 comments · Fixed by #3670
Assignees
Labels
modularization Part of Namada's modularization effort pre-mainnet Must happen before mainnet. prio:medium refactor / code quality

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Nov 7, 2023

At the moment, we split the codebase into separate directories and sub-packages based on technical aspects:

  • apps/ includes CLI code, wallet code, and a bunch of ABCI handling code
  • core/ includes types, storage layouts, and some miscellaneous state machine logic
  • ethereum-bridge/ contains a bunch of Ethereum bridge-related code
  • proof-of-stake/ contains a bunch of proof-of-stake related code
  • sdk/ contains the SDK code (client code)
  • shared/ contains validity predicate logic, miscellaneous types and events logic, some protocol structure, and some WASM VP wrapper functions
  • tx_prelude, vp_prelude, and vm_env contain some WASM standard library code (as far as I can tell)

At least personally, I find this separation somewhat confusing, for a few reasons:

  • it's difficult to find all of the code associated with a particular feature (e.g. governance), since it's split into many places
  • it's not always entirely clear whether some piece of code is in the right place, and some locations seem inconsistent
  • I think it probably creates more file conflicts than necessary, since code for unrelated features is not always clearly abstracted

I propose for discussion a rework of the structure of the codebase focused on feature modules, e.g.:

  • A module is a defined interface which must implement a bunch of functions, such as:
    • a validity predicate
    • logic to be run at the beginning/end of blocks/epochs
    • transaction builder logic for the sdk
    • commands to expose in the cli
    • genesis fields required
  • Governance/PGF, proof-of-stake, IBC, the Ethereum bridge, MASP all become modules
    • if convenient, these can be implemented as separate crates in some modules/ directory
    • all of these modules should implement this interface
    • no feature-specific code should live outside its module

Some precedent for this abstraction can be found in the modules system of the Cosmos SDK, which, although it has a lot of boilerplate, implements this basic module abstraction in a reasonably clean manner.

I think that if we do this well, it could make the codebase much easier to navigate, reduce file conflicts, and make it easier to add new modules in the future. Commentary & discussion welcome.

The dependency graph from 0.31.8:
deps
(generated with cargo depgraph --workspace-only --dedup-transitive-deps | dot -Tpng )

Module dependencies that should be removed:

  • trans_token <-
    • governance
  • parameters <-
    • governance
    • shielded_token
  • account <-
    • proof_of_stake
  • governance <-
    • proof_of_stake
    • ibc
  • token <-
    • ibc
  • proof_of_stake <-
    • ethereum_bridge
@sug0 sug0 added the modularization Part of Namada's modularization effort label Dec 15, 2023
@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 2, 2024

Looking over our current work, particularly @tzemanovic's PRs such as #2312, I thought it would be helpful to try to define the target dependency graph of our internal structure. Based on my understanding, it should look something like this (top -> bottom = deepest dependency -> shallowest):

Untitled-2024-01-12-1601

@tzemanovic @sug0 @batconjurer What do you think? Is this achievable?

@cwgoes cwgoes added the pre-mainnet Must happen before mainnet. label Feb 5, 2024
@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 7, 2024

Updated diagram per discussion with @tzemanovic @batconjurer

updated-diagrams

Agreed on order of operations:

  1. General modularization & separation of modules with a clear interface.
  2. Remove unnecessary WASM usage (specifically in MASP and IBC, which will both change to fully-native VPs).
  3. Reassess and consider further changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modularization Part of Namada's modularization effort pre-mainnet Must happen before mainnet. prio:medium refactor / code quality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants