Cleaner directory structure

[ebuchman]

Right now we have a ton of diverse functionality directly under src. Might be better to organize this a bit better, for instance, at a high level:

src/
  abci/
    abci/
    abci.rs
  amino/
    amino_types/
    amino_types.rs
  types/
    block/
    block.rs
    chain/
    chain.rs
    consensus
    consensus.rs
    evidence.rs
    genesis.rs
    hash.rs
    time.rs
    timeout.rs
    validator.rs
    version.rs
    vote/
    vote.rs
  config/
    config/
    config.rs
  crypto/
    merkle.rs
    private_key.rs
    public_key.rs
    signature.rs
  error.rs
  lib.rs
  p2p/
    channel/
    channel.rs
    moniker.rs
    net.rs
    node
    node.rs
    secret_connection/
    secret_connection.rs
  rpc/
    rpc/
    rpc.rs
  serializers.rs

Then for things like

  amino/
    amino_types/
    amino_types.rs

and

  abci/
    abci/
    abci.rs

we can probably consolidate further.

[tac0turtle]

I believe the case can be made here to use Cargo.toml in each folder as it will work better for tendermint/tendermint#3176 and the goal of go-tendermint. In this case each "reactor" can be treated as a module. I understand it will may lead to some dependancy headaches, but I believe it is a better approach for the direction that GO-TM is trying to move towards.

[tarcieri]

@marbar3778 yep, the whole thing can be a Cargo workspace with each folder representing an individual crate. They can be versioned together (e.g. cargo release --all) or separately depending on which actually makes sense.

[thanethomson]

Makes sense 👍 We should capture this in an ADR (#19).

[greg-szabo]

As of discussion on Sep 6: let's open a new repository for a "from scratch" version of tendermint Rust and implement that directory structure (logic structure skeleton) there. We discussed using crates with workspaces and adding all functionality as a skeleton. (Then using the current codebase to refactor and rethink into the new repo.)

[greg-szabo]

Referencing #19, Thane's idea for ADRs. Let's implement them in the new directory structure / repo:

In light of recent discussions regarding the spec and ADR processes for Tendermint, would it make sense to have a docs or adr folder in the tendermint-rs repo to allow us to discuss and record architectural decisions specific to the Rust implementation?

An example of what would require such an ADR: in implementing a reactor-style model in a similar manner to the Go version of Tendermint, should we roll our own again or should we make use of some kind of Tokio-based framework such as Actix? And if we go with a framework, how do we create/design/document some kind of interface to it that doesn't lock us into using that particular framework?

[ebuchman]

I don't think we need a new repo. We seem to have 3 upcoming concerns:

1. Maintain existing tendermint-rs roughly as is for the KMS and rpc client users
2. Lite Client
3. Reactor Framework

For (1), we should support the current tendermint-rs as necessary, but don't necessarily need to re-org the internal directory structure. The secret-connection should probably be broken out into a crate in the KMS repo though - #21 (comment)

For (2) we should make a new lite crate to define lite client traits and the relevant functionality in a way that is generic over the underlying data types and serialization. We'll want (something like) the existing tendermint-rs types to be able to implement these lite traits, and we should add a lite binary that reads from the rpc and performs a lite client sync.

For (3), we should make a new reactors crate to experiment with the reactor framework and components of specific reactors

I think working in this way should give us a better sense of if/when it will really help to refactor the tendermint-rs crate itself.

We should probably summarize this and the current architectural state in an ADR

[tarcieri]

@ebuchman if you'd like me to refactor the crate to match the directory structure you proposed, I wrote it and I'm an IntelliJ refactoring wizard, so it wouldn't take me that long

[ebuchman]

Lol thanks for the offer. Do you think it's worth it? Also can you clarify the intention with having eg. abci.rs and abci/, and would we need to preserve that? Also not sure if each of the directories here should be its own crate yet. I think maybe there are 5 crates here:

• crypto
• tendermint-rs (depends on crypto)
• config (depends on tendermint-rs)
• rpc (depends on tendermint-rs)
• secret-connection (move to KMS - does it even need to depend on anything here?)

Probably worth considering the above and getting something in an ADR re the overall architecture before we refactor

[tarcieri]

Moving secret-connection back to the KMS repo sounds like a start

[ebuchman]

@tarcieri is there a reason you used the eg.

vote.rs
vote/

pattern for modules vs using a more standard

vote/
  mod.rs

pattern ? We were discussing with @xla and @liamsi moving to the latter as part of this refactor.

[tarcieri]

• vote/mod.rs was the convention in the 2015 module system
• vote.rs is the new convention in the 2018 module system (a.k.a. "path clarity")

Moving to the former may break your code in a future edition when the 2015 convention is eventually deprecated.

[xla]

@tarcieri Thanks for the clarification, this is insightful.

[ebuchman]

Re the above:

 I think maybe there are 5 crates here:

- crypto
- tendermint-rs (depends on crypto)
- config (depends on tendermint-rs)
- rpc (depends on tendermint-rs)
- secret-connection (move to KMS - does it even need to depend on anything here?)

Some thoughts/followup/questions here:

• secret connection has been moved out (woo!)
• Should crypto and config really be their own crates? how do we decide? config doesn't have any unique dependencies and crypto will always be needed by the tendermint crate. I guess the question is if there's a use case for importing crypto without importing the rest of the tendermint crate?
• Does separating the rpc module into its own crate solve Feature guard the networking stuff #289
• We should move rust-abci in here as it's own crate cc Absorb rust-abci #29
• We now have light-client and light-node - should these be separate? Is there a way to combine the lib and the binary so that folks who only want the lib don't need to worry about the binary (which has way more dependencies) or do we need to keep them as separate crates?

[tarcieri]

Re #289, the RPC client is the only thing in the tendermint crate that, to my knowledge, actually does any networking, so extracting it into a separate crate should remove hard dependencies on things like tokio for users who just want the core types in the tendermint crate.

[liamsi]

We now have light-client and light-node - should these be separate?

I think it's OK to have one crate to use the light client as a library and one as a binary. The binary crate should also commit a Cargo.lock file: https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries
I think, the two could also be merged if that is desired though. I prefer a clear boundary between both.

That said, there is also still that lite module which should be merged with light-client (my understanding some code was copy & pasted from lite and it might be partly obsolete now).

[xla]

That said, there is also still that lite module which should be merged with light-client (my understanding some code was copy & pasted from lite and it might be partly obsolete now).

Do you have more context to that? Maybe PRs or other issues?

[xla]

Re #289, the RPC client is the only thing in the tendermint crate that, to my knowledge, actually does any networking, so extracting it into a separate crate should remove hard dependencies on things like tokio for users who just want the core types in the tendermint crate.

Addressed in #338

[tarcieri]

@xla fantastic! I can definitely make use of that in the KMS to selectively enable RPC for the features that need it

[romac]

That said, there is also still that lite module which should be merged with light-client (my understanding some code was copy & pasted from lite and it might be partly obsolete now).

Yes, we plan to deduplicate the code once the light client is finalized, and thus perhaps eventually drop the tendermint::lite module altogether.

[liamsi]

An alternative to the current light-client and light-node naming of the crates could be:
light-client
light-client-cli (instead of light-node) ?

[xla]

It could be an alternative yet leaves room for interpretation as a cli could be utility to interact with a binary offering light client functionality. Usually node or daemon are more indicative.

[liamsi]

We could also consider putting the integration tests in a separate crate:
https://github.com/informalsystems/tendermint-rs/tree/master/tendermint/tests

Then, we could get rid of the dev-dependency to rpc inside of tendermint-rs.

[thanethomson]

Is our current directory structure clean enough? @ebuchman

[ebuchman]

Probably not :). There's obviously been lots of progress, especially with the serialization cleanup, but the tendermint crate is still a jumble of top level modules with minimal organizational structure and there are a few other good comments here. I'll open a new issue to capture