cosmos / ibc-rs

Rust implementation of the Inter-Blockchain Communication (IBC) protocol.
Apache License 2.0
182 stars 74 forks source link

imp: feature gate `ibc` exports #1138

Closed rnbguy closed 3 months ago

rnbguy commented 3 months ago

Improvement Summary

The top ibc crate exports everything in the repo. This may not be intended, especially in the CosmWasm context. (See cosmos/rollkit-ibc#15). Currently, this can be avoided by importing the sub-crates directly. This may be a bit messy to maintain as a user has to maintain a consistent version across the sub-crates.

Proposal

We can introduce features for each sub-crate:

[features]
core = ["dep:ibc-core"]
clients = ["dep:ibc-clients"]
primitives = ["deps:ibc-primitives"]
# ... for each sub-crate dependency
std = [
    "ibc-core?/std",
    "ibc-clients?/std",
    "ibc-primitives?/std",
]
serde = [
    "ibc-core?/serde",
    "ibc-clients?/serde",
    "ibc-primitives?/serde",
]
# ... for each feature

cargo-doc documents the usage of "package-name?/feature-name".

With this, now we can

ibc = { version = "0.50.0", features = ["core", "primitives", "serde"] }

Which is semantically equivalent to,

ibc-core = { version = "0.50.0", features = ["serde"] }
ibc-primitives = { version = "0.50.0", features = ["serde"] }
Farhad-Shabani commented 3 months ago

The meta ibc crate is mainly for host chain integrators, offering core/clients/apps implementations. We broke down ibc into multiple libraries for selective imports, benefiting developers like CosmWasm devs who can now import sub-crates directly as you mentioned. I'm cautious about feature gating as it could still lead to excessive access (like access to the connection layer while CW implementors do not need that) and compatibility issues. Besides, These components are not features but essential parts of ibc that should be available to host integrators without requiring them to enable/disable them. IMO, we should focus on improving our documentation. By providing clear guidance on how to import relevant libraries, we can ensure that each group of users can access what they need.

rnbguy commented 3 months ago

... lead to excessive access (like access to the connection layer while CW implementors do not need that) and compatibility issues.

Can you give an example of compatibility issues? I am curious.

... These components are not features but essential parts of ibc that should be available to host integrators without requiring them to enable/disable them.

Agree with this.

Farhad-Shabani commented 3 months ago

Can you give an example of compatibility issues? I am curious.

All good now. I used 'connection' as an example layer unrelated to light client development, where future introduction of other dependencies, especially transitive ones not necessarily supported by CosmWasm, may occur.

rnbguy commented 3 months ago

I see. Anyway, I see your point.

Essentially, there should be packages like ibc-L1, ibc-cw, ibc-sovereign, etc. And, the current ibc package is the ibc-L1.

Feel free to close the issue.

Farhad-Shabani commented 3 months ago

Essentially, there should be packages like ibc-L1, ibc-cw, ibc-sovereign, etc. And, the current ibc package is the ibc-L1.

I see. It can be a good idea. Should think of.

By the way, thank you @rnbguy for tracking down the Rollkit integration issue!