informalsystems / hermes

IBC Relayer in Rust
https://hermes.informal.systems
Apache License 2.0
434 stars 319 forks source link

Separate internal and public crate APIs using explicit export list #2188

Open soareschen opened 2 years ago

soareschen commented 2 years ago

Summary

Move implementation code into crate::internal, and explicitly export public APIs at the top level.

Problem Definition

Rust's approach for hiding private APIs and exposing public APIs is not very flexible when projects are split into multiple crates. The pub (crate) modifier only allows an internal interface to be exposed to other constructs within the same crate. But we cannot easily state that a certain API is accessible to an external crate managed by the same project.

This creates issues if we want to share internal APIs between different crates like ibc, ibc-relayer, and ibc-test-framework. While making the constructs public could solve the issue, this may potentially introduce maintenance burden if other projects decide to use the internal APIs.

A solution to this is to introduce an internal submodule that is public, but do not guarantee API stability. This way, crates within the same project can import constructs from the internal submodule, but external users will be adviced against importing the internal submodule.

At the top level, we then make explicit decision on which constructs from the internal crate should be exported. The explicit export list also makes it easier to conduct reviews. Reviewers can be aware of a new public API being added by just looking at lib.rs, instead of looking for the pub keyword scattered across many files.

The use of internal module is a common convention in Haskell. It also allows power users who are not afraid of API breakage to import internal constructs at the own risk, without affecting regular users who prefer stable APIs.

Proposal

Move all code into an internal module/subdirectory, and explicitly export public APIs in lib.rs.


For Admin Use

adizere commented 2 years ago

Is this something you would recommend we fix before we release v1 for the relayer* crates?

For ibc crate we'll remain on the 0.* line for slightly longer (>6 months at least), so API instability is not a concern yet.

soareschen commented 1 year ago

Yes, it should be done before a stable API release.

hu55a1n1 commented 1 year ago

We might also want to consider ideas from -> https://rust-lang.github.io/api-guidelines/future-proofing.html, such as using Sealed traits, before releasing relayer v1.

romac commented 1 year ago

I thought that we were only planning on releasing the ibc-relayer-cli crate as v1, but not the other crates?