celestia-node icon indicating copy to clipboard operation
celestia-node copied to clipboard

`Node` pkg refactoring

Open renaynay opened this issue 2 years ago • 1 comments

  • [ ] 1. Split components into component-specific subpackages as fx.Modules (e.g. header components go into node/header)
  • [ ] 2. Split opts and configs into related subpackages
  • [ ] 3. Create node subpackage inside node directory (e.g. node/node) that provides its own fx.Module
  • [ ] 4. Migrate cmds + flags into node/<subpkg> and have cmd depend on them
  • [ ] 5. https://github.com/celestiaorg/celestia-node/issues/965

Desired outcome:

module.Cmds
module.Flags
module.API
module.Module
module.Config
module.Init 
header.Module(node.Type) --> provides header.Module based on node type

renaynay avatar Jul 28 '22 13:07 renaynay

easiest -> hardest

create module.Module (fx.Module)
put module config in subpackage module.Config
cmds and flags
implementing `type <pkg_API> interface` inside subpackage

renaynay avatar Aug 11 '22 10:08 renaynay

I am starting to become really conflicted about how the interface between the cmd/flags and the node subpackages should look.

These flags and their parsing are clearly logically closer to the command themselves than the actual implementation of the options. It seems like a bad pattern to have the subpackages themselves modifying the "global" state (by handling setting the Env themselves).

I think the best solution (for now) may be to keep the flags and their parsing methods in cmd, but to move the options and configs to the subpackages. This is what Hlib is experimenting with in https://github.com/renaynay/celestia-node/pull/90 and I really like it.

I have spent quite a bit of time now trying to figure out the best way to move the flags into the subpackages themselves, but will abandon this and move forward with only moving the opts and configs, and creating modules for the individual subpackages - based on top of Hlib's PR

distractedm1nd avatar Aug 24 '22 07:08 distractedm1nd

Eventually™️, we still should find a way for any module to hold the set of flags and cmds relatated to the the module. This will be especially important once we define APIs for modules with RPC server/client shims, so that introduced cmds can interact with the runnin node over RPC.

E.g. there is a p2p module, which contains an API with ListenAddr method(from #506) and respectively there is a command celestia p2p listen-addr which calls the ListenAddr over the API. It also has a set of flags and options for the method. The p2p module here encompasses everything mentioned and all other p2p related commands with their flags.

In our case right now, we have onlu flags for the start/run command which is slightly different case from the p2p specific commands mentioned above. Ideally, we would also keep these start/run p2p related flags in the p2p module, but it might be a bit premature to do this now.

Wondertan avatar Aug 24 '22 13:08 Wondertan

Moving 4. Migrate cmds + flags into node/<subpkg> and have cmd depend on them into a separate issue

renaynay avatar Sep 23 '22 13:09 renaynay

Closed by #997

renaynay avatar Sep 27 '22 09:09 renaynay