Decode ClientState from Protobuf's Any type #233
Conversation
| use crate::mock_client::header::MockHeader; | ||
| #[cfg(test)] | ||
| use crate::mock_client::state::{MockClientState, MockConsensusState}; | ||
| use { |
There was a problem hiding this comment.
Ahh this is such a treat to the eyes.
| impl AnyClientState { | ||
| pub fn from_any(any: prost_types::Any) -> Result<Self, Error> { | ||
| match any.type_url.as_str() { | ||
| "ibc.tendermint.ClientState" => { |
There was a problem hiding this comment.
Minor:
Maybe we should consider keeping these strings (lines 125 "ibc.tendermint.ClientState" and 135 "ibc.mock.ClientState") hardcoded in some enum as opposed to hardcoded here? I can imagine they could even go in the ibc-proto repo, not here, but not sure what would be best.
To keep things simpler and get this merged fast, we can also just keep the hardcoded values as they are and just add a todo.
There was a problem hiding this comment.
Yeah good point! Those should probably be defined as constants somewhere.
| @@ -14,23 +14,26 @@ | |||
| //! TODO: Separate the Cosmos-SDK specific functionality from canonical ICS types. Decorators? | |||
|
|
|||
| #![allow(clippy::too_many_arguments)] | |||
There was a problem hiding this comment.
Looking at the code, I think it's safe to remove this now.
| @@ -1,5 +1,6 @@ | |||
| use crate::ics23_commitment::commitment::CommitmentPrefix; | |||
| use ibc_proto::commitment::MerklePath; | |||
|
|
|||
| use ibc_proto::ibc::commitment::MerklePath; | |||
There was a problem hiding this comment.
Seems we always import types from ibc proto as use ibc_proto::ibc::X, so I'm wondering why is the ibc namespace in the middle necessary. We could just say use ibc_proto::X.
There was a problem hiding this comment.
See this PR for a (poorly explained) rationale: informalsystems/ibc-proto#9
There was a problem hiding this comment.
Also, the ibc_proto crate exports other types which are not ibc-specific so imho it makes sense to qualify their usage, though we could te-export them at the top level but that may cause similar issues to the one mentioned in the PR linked above.
| impl AnyClientState { | ||
| pub fn from_any(any: prost_types::Any) -> Result<Self, Error> { | ||
| match any.type_url.as_str() { | ||
| "ibc.tendermint.ClientState" => { |
There was a problem hiding this comment.
Wondering if we should define these type_url strings in a separate place so they can also be used for encoding. We can do it later also.
There was a problem hiding this comment.
Agreed! See my reply to Adi’s comment above.
* initial changes for ibc_proto 0.2.2 * Fix cargo fmt errors * Add client proof to verify proofs * Add the client state verification functions * Fix error * Fix fmt error * Review comments * Some initial changes verification arguments not tight to ClientDef * Decode ClientState from Protobuf's Any type (#233) * Fix Clippy warnings * Add stub for deserializing ClientState from Protbuf's Any * Update to latest ibc-proto * Decode Tendermint client state from raw * Decode raw client states * Add FIXME comments to uses of `epoch_height` * Update ibc-proto to 0.3.0 * Remove local path to ibc-proto dependency * Stylistic fixes * cleanup, comment is in the trait * fix mock context historical info validation Co-authored-by: Romain Ruetschi <romain@informal.systems>
See: cosmos/ibc-rs#115
Description
Decode both Tendermint and Mock client states from Protobuf's Any type.
For contributor use:
docs/) and code commentsFiles changedin the Github PR explorer