conorsch
commented
1 year ago Use the TypedEvent trait we recently upstreamed into tendermint-rs to have ibc-types act as a single source of truth for the encodings of structured data in ABCI events used by the IBC implementation to notify relayers about IBC data, folding the event construction from our IBC component into the ibc-types crate.
Done in the ibc-types repo, but we haven't updated the ibc create in the penumbra repo to match yet.
Is your feature request related to a problem? Please describe.
Penumbra has its own implementation of IBC, designed around our asynchronous chain state abstractions. Historically, this implementation has been built on top of
ibc-rs, using theibc-rstypes for modeling IBC messages and some standalone, stateless verification methods.In parallel, the
ibc-rsteam at Informal has been working on filling outibc-rsto include a complete implementation of IBC. This implementation is initially intended for Anoma, and is tailored to its state model. We can't use it, because Penumbra's chain state is accessed asynchronously, so it needs anasyncAPI, and because it's gone through significant churn on the way to discovering the right abstractions for its initial use-case (Anoma), just as our IBC implementation has gone through churn on the way to our discovering the right abstractions for our state model.In discussions with the
ibc-rsteam over the last year, we highlighted our view that the right move was to collaborate on a common set of domain types for modeling IBC messages, and work towards deeper integration as codebases stabilize. Historically, this has been fine, because IBC messages themselves are specified as part of the IBC protocol, and so are relatively insulated from implementation churn.However, in February, the
ibc-rsteam at Informal let us know that they were planning to remove the IBC message types entirely, and shiftibc-rsfrom a general-purpose library to only provide an opaque IBC implementation. We let them know this would break our implementation, and they told us that they "cannot modifyibc-rs's direction" and suggested forking the code, as they'd done with Hermes, Informal's IBC relayer, when its needs also conflicted with theibc-rsdirection. (Hermes now maintains its own entirely distinct library for modeling IBC messages).Since then, the rate of breaking changes to the parts of the crate we actually use has accelerated, and with the
0.37revision, it's now no longer possible for us to upgrade (details in #2374). This blocks forward progress on our own IBC implementation, because it means we can't consume the ICS23 extensions we recently upstreamed that allow support for non-inclusion proofs for sparse merkle trees (as used in Penumbra, Anoma, Celestia, ...).As a result, we need to fork the crate. This isn't a desirable outcome, but we also need our code to work.
Our hope is that over time, we can pull out minimally-scoped components bottom-up, and hope that they can be reused by different projects. This strategy has worked well so far with the ABCI domain types (upstreamed into
tendermint::abci), thetower-abcicrate (used by Anoma and others), thejmtcrate (now co-developed with Sovereign Labs), and thepenumbra-storagecrate (used by at least one unreleased project in development).Describe the solution you'd like
In the near-term, we need to rescue the IBC implementation and restore its ability to consume ecosystem upgrades. Our fast path to doing this is:
ibc-rsat0.29.0, the last version we know to work with Penumbra, and publish asibc-types(which will eventually just be a common set of data types, not an IBC implementation). Done in https://github.com/penumbra-zone/ibc-typesAt this point, we're back to where we started before the breakage, and can continue working on Penumbra.
In the medium-term, we extract and develop a minimally-scoped, generally useful version of
ibc-types:ibc-typescrate to delete all of the module implementations and strip it down to just domain types and events.create_spanmethod that allows attaching atracing::Spanto the processing of an IBC message.TypedEventtrait we recently upstreamed intotendermint-rsto haveibc-typesact as a single source of truth for the encodings of structured data in ABCI events used by the IBC implementation to notify relayers about IBC data, folding the event construction from our IBC component into theibc-typescrate.ibc-typesinto the Penumbra repo'sibccrate.At this point,
ibc-typesmeets our needs, and hopefully it fills the gap in the ecosystem of having a common library for modeling IBC messages, for whatever purpose, and we could explore having either Hermes oribc-rsdepend on it and collaborate on its development, if either team were so inclined.In the longer-term, we continue to extract our IBC implementation into a standalone, Penumbra-independent crate providing an async IBC implementation:
penumbra-ibccrate depending onpenumbra-storage;ibc-typesor another crate so that they can be shared between IBC implementations;penumbra_storage::{StateRead, StateWrite}traits with an abstract state-access trait, designed to be instantiated with thepenumbra_storagetraits but not dependent on them;At this point,
ibc-asyncis hopefully useful as a general-purpose IBC implementation, depending on a common set of modular components that could also be used byibc-rs, if that team was so inclined.Describe alternatives you've considered
We don't have a lot of good alternatives here.
We could use the opaque
ibc-rsinterfaces, despite their being synchronous. This would require writing a lot of complex Rust code to pass in and out of async, passingtokio::Runtimehandles around, etc., but all of that engineering effort is wasted on papering over an abstraction mismatch, rather than producing any useful tooling for the ecosystem.We could adopt a hypothetical future async interface to
ibc-rs. This interface doesn't currently exist, but our code is broken now.Neither of these options are particularly compelling.