ouroboros-network icon indicating copy to clipboard operation
ouroboros-network copied to clipboard

Split ouroboros-network & ouroboros-consensus packages

Open coot opened this issue 5 years ago • 9 comments
trafficstars

ouroboros-consensus package is quite large, and whenever the networking team needs to update it, we suffer from re-compiling a lot of code. The parts that depend on ouroboros-network are twoards the end of compilation pipeline.

I sugest introducion ouroboros-consensus-node package where the integration between ouroboros-network and ouroboros-cosnensus is made. We could also split ouroboros-network so that the ouroboros-cosnensus package would only depend on the mini protocols, chain fragments etc, but not on p2p-governor, diffusion and other rather low level networking details.

coot avatar Sep 17 '20 11:09 coot

The goal would be to end up with the ouroboros-consensus main lib not depending on -network at all anymore.

edsko avatar Sep 17 '20 12:09 edsko

Network would be happy with reduced compilation times and I would be happy with improved independency and an improved understanding of what exactly we depend on, and where.

edsko avatar Sep 17 '20 12:09 edsko

I'd be happy with cleaner dependency tree too :).

coot avatar Sep 17 '20 12:09 coot

I'm all for it!

mrBliss avatar Sep 28 '20 08:09 mrBliss

Some suggestions for names:

  • ouroboros-common: for the modules that both network and consensus need, e.g., Ouroboros.Network.Block, Ouroboros.Network.ChainFragment, etc. We can use the Ouroboros.* or Ouroboros.Common.* module hierarchy.
  • ouroboros-node: for the modules that combine network with consensus and the node-related stuff. We can use the Ouroboros.Node module hierarchy. I think it would be best if there is no dependency (in either direction) between ouroboros-node and the various ledger integration packages (ouroboros-consensus-byron, -shelley, -cardano, -mock, ...). This would be good for "low coupling" but also for build speed.

I'm looking forward to do this reorganisation! (@edsko just for the contribution stats :smile:)

In consensus, we could also consider splitting up the storage related-code into a separate package, but I think this will be detrimental to the build speed since the ChainDB relies on most of the non-storage code anyway (as it does chain selection and ledger validation). So if we split off storage in a separate package, cabal must first compile the non-storage consensus code before it can start on the storage package, whereas now it can parallelise this when the dependencies allow it.

mrBliss avatar Oct 02 '20 08:10 mrBliss

@edsko pointed out that it might make sense to change the ouroboros- prefix to cardano- (e.g., cardano-network and cardano-consensus) which is not a bad idea. We should then also change the repo name, which will be a PITA, though.

However, the fact that we have ouroboros-consensus-cardano (cardano-consensus-cardano? :slightly_frowning_face:) might indicate that cardano- is not the right prefix after all :thinking:

mrBliss avatar Oct 02 '20 08:10 mrBliss

There's nothing specific to cardano in ouroboros-network but there's also nothing specific to ouroboros either. We could use some fancy name instead.

What about ouroboros-chain instead ouroboros-common?

Another idea for ouroboros-node could be ouroboros-diffusion-node: it combines diffusion with consensus (though I am not convinced that node reflects what consensus is about), but maybe one will came up with a better idea in this direction.

coot avatar Oct 02 '20 15:10 coot

There's nothing specific to cardano in ouroboros-network but there's also nothing specific to ouroboros either. We could use some fancy name instead.

Yeah, Edsko and I realised too that cardano would not be ideal. For example, the Midnight team and soon the ETC2/Mantis teams use this codebase too.

What about ouroboros-chain instead ouroboros-common?

Yeah, I think that can work for me. If the majority of modules/types that actually end up in that package are not really related to "chain", then I would hesitate, but I don't think that will be the case.

Another idea for ouroboros-node could be ouroboros-diffusion-node: it combines diffusion with consensus (though I am not convinced that node reflects what consensus is about), but maybe one will came up with a better idea in this direction.

Hmm, I'm not convinced. Personally, "diffusion" just makes me think of the Diffusion stuff in the network layer, which doesn't have the same meaning for me as for you. I'd like something that conveys: "combining the network layer with the consensus layer into a functioning node".

mrBliss avatar Oct 02 '20 16:10 mrBliss

Thought: we could rename ouroboros-consensus-cardano to cardano-consensus.

I think it would also be good for consensus to have some API-like modules that export all things downstream users might need. For example, re-export all storage-related things from Ouroboros.Consensus.Storage, re-export all Shelley-related things from Ouroboros.Consensus.Shelley.

mrBliss avatar Oct 13 '20 09:10 mrBliss