Repo Restructing

[MSevey]

Overview

As we approach v1, we have begun writing specifications for rollkit. In this process of writing specs, the topic of how the code is currently structured and its impacts on the specifications and implementations has come up.

This discussion is aimed at deciding what, if any, code restructuring we want to do that will lead to an overall improvement.

Key factors:

• Dependency management
• Specifications vs implementation
• Golang best practices
• Ecosystem standards

Golang Standard Layout

REF: https://github.com/golang-standards/project-layout

This is a basic layout for Go application projects. It's not an official standard defined by the core Go dev team; however, it is a set of common historical and emerging project layout patterns in the Go ecosystem. Some of these patterns are more popular than others. It also has a number of small enhancements along with several supporting directories common to any large enough real world application

Ecosystem Standards

Rollkit is meant to be a very agnostic framework. However, v1 is highly based on the cosmos eco system. As such, we should consider how the cosmos-sdk is structure.

REF: https://github.com/cosmos/cosmos-sdk

Current Structure

assets/
block/
config/
conv/
da/
docker/
docs/
libs/
log/
mempool/
mocks/
node/
p2p/
proto/
rpc/
specs/
state/
store/
third_party/
types/

Initial Review

assets

This directory holds website assets and other images and is the recommended convention.

block

This directory contains things like block caching, block syncing, and block aggregation (sequencing).

config

This directory has combination of configs and defaults, and is the recommended convention.

conv

This directory has some helper functions for converting types. The conv directory is not a standard directory in either the golang or cosmos-sdk examples. More often this type of tooling is under the types directory as it is converting one type to another. Consolidating the type definition and conversion code would help with organization and reduce dependency scope.

da

This directory holds code related to connecting to the DA layer. In the future, rollkit want to support multiple DA layers, sequencers, and execution layers. If might make sense to put this in a pkg directory since it has a higher likelihood of being used externally.

docker

This holds a docker file for the mock DA layer. Since rollkit itself doesn't run and therefore doesn't have a dockerfile, it might make sense to more this to a testing directory or mock directory as to not cause confusion about the project and using docker.

docs

This directory has some specifications and ADRs in it. Since we have a specific docs repo for the rollkit.dev website, it might make sense to move this into the specs directory to make it clear that the documentation in the rollkit repo is technical specifications.

libs

This directory holds helpers for external repos. Currently this just celestia-app. Since this directory is for external tooling it might make sense to use the third_party convention to make it explicit that this is tooling for handling third part code.

log

This directory holds an implementation of the tendermint logger with a logger for use in testing. Since this is a third part tool, we should move it to the same directory as the other third part tools for consistency.

mempool

This directory holds the mempool related code. It might make sense to move this to an internal directory.

mocks

This holds code for the Application mock used in testing. mocks is not a common top level directory and since it is only used in testing, it might make sense to move under the test directory.

node

This director is a catch all for all node related activities and has mempool code, exchange/sync code, as well as full vs light node definitions.

p2p

This directory holds the p2p network logic for connecting to peers and gossiping transactions. It might make sense to move this to an internal directory.

proto

This top level directory holds the proto files for the project and follows the common practice of the cosmos-sdk.

rpc

This is a rpc server used by the cosmos-sdk

specs

This directory holds the specs website code. Typically this directory is called docs but we have a separate docs repo for rollkit.dev. Keeping the naming of specs helps clarify the type of documentation that lives in the rollkit repo vs the docs repo.

state

This directory holds the code related to handling the state, applying txs, storing, etc.

store

This directory is a general kv store tooling. This is copying the cosmos-sdk repo structure of the top level store directory. If there were more of this type of tooling a tools directory might be handy to house this type of code.

third_party

This directory currently holds some proto code and is the recommended convention for holding this type of code.

types

This directory holds the definitions and basic functions for some common types and follows the common practice of the cosmos-sdk.

Recommendations

As a general recommendation, if the reason for a package is not explicitly clear after writing the specs, or by the structure itself, we should add READMEs to the directory with a quick explanation of the directory.

Low Impact

1. Move all conv code to types package.
2. Remove libs and put celestia-app code in third_party
3. Move docs contents into specs directory since we have a separate repo for the rollkit docs. This would make it clearer the type of documentation that goes into rollkit/specs vs the docs repo.
4. Move log to third_part directory
5. Create a test directory
6. Move log/test to test/log
7. Move mocks under test
8. Move dockerfile under test

Result

assets/
block/
config/
da/
mempool/
node/
p2p/
proto/
rpc/
specs/
state/
store/
test/
  docker/
  log/
  mocks/
third_party/
  celestia-app/
  log/
  proto/
types/

Medium Impact

1. Align on planned use of top level directories, internal, pkg, x, or some combination.

Proposed restructure

assets/
config/
pkg/ or x/ or internal/
  da/
  block/
  mempool/
  node/
  p2p/
  state/
  store/
proto/
rpc/
specs/
test/
third_party/
types/

High Impact

1. Refactoring of node, block, and p2p??

Open Discussion Points

pkg and internal

One of the key differences between the cosmos-sdk layout and the golang standard layout is the use of pkg and internal. Essentially there is no internal directories and instead of pkg they use x/.

Rollkit is similar to cosmos-sdk in this way. However not having an internal or a pkg directory leads to an overloaded top level directory. There is a personal preference side of this but also there is a dependency management side of this as well.

From a dependency management point of view, there are a few ways of addressing potential circular dependencies, but the mostly revolve around directory structure and using appropriate top level directories.

Historically the team has had a preference of fully refactoring out challenging packages due to the circular dependencies with repos like celestia-app and celestia-node.

If the root cause of a circular dependency is a general type/struct that is used in the vast majority of directories, then using the types. If the root cause of a circular dependency is a more specific type/struct that is localized to just a few components, then restructuring those components under a common top level directory might make more sense from a developer experience.

[tzdybal]

celestia-app directory is a work around and temporary solution. We should get rid of it as soon as technically possible.

We discussed this before and this code should be extracted as a library, and it's blocked mostly by team priorities and capacity.

[MSevey]

I agree and it makes total sense, I remember the convo. We should still move it to third_party as a standard for any other similar cases in the future. Any third party code is in the same category to a degree. We can't directly use the third party code without some sort of modification. If we could just directly import it we would.

The same thing goes for the logger. If the logger was something we could just directly import, we would. But since we want to have some custom logic, we have the work around/implementation code.

[MSevey]

pkg / internal / x

Is there a technical reason coming from the use of the cosmos-sdk, that the packages for the components need to be in the top level directory of the repo?

Would do some sort of directory structure like this break anything?

pkg/ or x/ or internal/
  da/
  block/
  mempool/
  node/
  p2p/
  state/
  store/

[MSevey]

Dropping this.

[MSevey]

node / P2P / block refactoring

Looking into the code some more to understand the potential for refactoring I propose the following.

P2P

The P2P package should remain focused on just the P2P network, i.e. the management of connecting to peers.

Since P2P is such a core component of a blockchain, almost ever other package has some P2P component. Trying to keep everything P2P related under a P2P package would end up unfactoring 80% of the code.

So to keep the code organized, maintaining a minimalist approach of the P2P package is best imo.

node

The node package should be limited to the definition of a node and the functions that the node needs to before to make the blockchain operate. Code related to the definition of header and block exchange should be move to the block package.

block

The block package should be where all code related to the block creation, publication, and retrieval is stored.

Recommended Actions

1. node/mempool.go - this is defining a mempoolIDs struct. this file should be moved to the mempool package to keep all mempool structs together.
2. The following node/ files should be moved to block/:
   • block_exchange.go
   • exchange.go
   • header_exchange.go

Spec Impact

As a result of these changes, here is what the expectation of the specs would be.

• P2P spec - Specification defining the p2p network and how peers are discovered, connected to, removed, managed, etc
• node spec - Specification defining the types of rollkit nodes, light, full, sequencer, etc
• block spec - Specification defining a rollkit block, how it is created, published, retrieved, etc

[MSevey]

Low Impact implemented in #1217