Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use TryFrom to map protobuf data structures to our domain types #132

Closed
ebuchman opened this issue Jul 14, 2020 · 1 comment · Fixed by informalsystems/hermes#152
Closed
Assignees

Comments

@ebuchman
Copy link
Member

ebuchman commented Jul 14, 2020

We currently have custom rolled domain types for the ConnectionEnd, ChannelEnd, etc. We also have the generated protobuf types. We should follows the convention adopted in the tendermint-rs repo that uses the protobuf types as an intermediary for deserializing from the wire into our domain types. We can do this by implementing TryFrom on our domain types to populate them from the protobuf types. Then with some serde magic, the deserialization can automatically flow through the protobuf types into our types.

See for instance the CommitSig and RawCommitSig in Tendermint:

https://github.com/informalsystems/tendermint-rs/blob/aefb3e062bb9026d765db509a676a9492b3c23f2/tendermint/src/block/commit_sig.rs#L11-L13

https://github.com/informalsystems/tendermint-rs/blob/aefb3e062bb9026d765db509a676a9492b3c23f2/tendermint/src/serializers/raw_commit_sig.rs#L22-L37

@greg-szabo
Copy link
Member

informalsystems/hermes#152 fixes that with the same pattern but slightly different tools (because prost doesn't support serde).

CommitSig uses serde annotation in the domain type to identify the raw serde-decoded type and the TryFrom trait to validate the data from the raw type into the domain type.

ConnectionEnd (and others in ibc-rc) uses a "TryFromRaw" custom trait that does both: it works similarly to TryFrom as in it has a try_from(raw) method but it also introduces an associated type to store what exactly that raw type is. (Which in the previous case came from the serde annotation.)

@hu55a1n1 hu55a1n1 transferred this issue from informalsystems/hermes Sep 29, 2022
livelybug pushed a commit to octopus-network/ibc-rs that referenced this issue Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants