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

Decode ClientState from Protobuf's Any type #233

Merged
merged 10 commits into from
Sep 17, 2020
4 changes: 2 additions & 2 deletions modules/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ description = """

[dependencies]
tendermint = "0.15.0"
tendermint-rpc = { version = "0.15.0", features=["client"] }
tendermint-rpc = { version = "0.15.0", features = ["client"] }

# Proto definitions for all IBC-related interfaces, e.g., connections or channels.
ibc-proto = "0.2.2"
ibc-proto = "0.3.0"

anomaly = "0.2.0"
thiserror = "1.0.11"
Expand Down
2 changes: 1 addition & 1 deletion modules/src/address.rs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
pub struct AccAddress(Vec<u8>;
pub struct AccAddress(Vec<u8>);
48 changes: 41 additions & 7 deletions modules/src/ics02_client/client_def.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
use prost::Message;
use serde_derive::{Deserialize, Serialize};

use crate::downcast;
use crate::ics02_client::client_type::ClientType;
use crate::ics02_client::error;
use crate::ics02_client::error::{self, Error};
use crate::ics02_client::header::Header;
use crate::ics02_client::state::{ClientState, ConsensusState};
use crate::ics03_connection::connection::ConnectionEnd;
use crate::ics07_tendermint as tendermint;
use crate::ics07_tendermint::client_def::TendermintClient;
use crate::ics07_tendermint::client_state::ClientState as TendermintClientState;
use crate::ics23_commitment::commitment::{CommitmentPrefix, CommitmentProof, CommitmentRoot};
use crate::ics24_host::identifier::{ClientId, ConnectionId};
use crate::try_from_raw::TryFromRaw;

use ibc_proto::ibc::tendermint::ClientState as RawTendermintClientState;

use ::tendermint::block::Height;

#[cfg(test)]
use crate::mock_client::client_def::MockClient;
#[cfg(test)]
use crate::mock_client::header::MockHeader;
#[cfg(test)]
use crate::mock_client::state::{MockClientState, MockConsensusState};
use {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh this is such a treat to the eyes.

crate::mock_client::client_def::MockClient,
crate::mock_client::header::MockHeader,
crate::mock_client::state::{MockClientState, MockConsensusState},
ibc_proto::ibc::mock::ClientState as RawMockClientState,
};

pub trait ClientDef: Clone {
type Header: Header;
Expand Down Expand Up @@ -63,6 +69,7 @@ pub trait ClientDef: Clone {
) -> Result<(), Box<dyn std::error::Error>>;

/// Verify the client state for this chain that it is stored on the counterparty chain.
#[allow(clippy::too_many_arguments)]
fn verify_client_full_state(
&self,
_client_state: &Self::ClientState,
Expand Down Expand Up @@ -106,12 +113,39 @@ impl Header for AnyHeader {

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub enum AnyClientState {
Tendermint(crate::ics07_tendermint::client_state::ClientState),
Tendermint(TendermintClientState),

#[cfg(test)]
Mock(MockClientState),
}

impl AnyClientState {
pub fn from_any(any: prost_types::Any) -> Result<Self, Error> {
match any.type_url.as_str() {
"ibc.tendermint.ClientState" => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good point! Those should probably be defined as constants somewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! See my reply to Adi’s comment above.

let raw = RawTendermintClientState::decode(any.value.as_ref())
.map_err(|e| error::Kind::ProtoDecodingFailure.context(e))?;
let client_state = TendermintClientState::try_from(raw)
.map_err(|e| error::Kind::InvalidRawClientState.context(e))?;

Ok(AnyClientState::Tendermint(client_state))
}

#[cfg(test)]
"ibc.mock.ClientState" => {
let raw = RawMockClientState::decode(any.value.as_ref())
.map_err(|e| error::Kind::ProtoDecodingFailure.context(e))?;
let client_state = MockClientState::try_from(raw)
.map_err(|e| error::Kind::InvalidRawClientState.context(e))?;

Ok(AnyClientState::Mock(client_state))
}

_ => Err(error::Kind::UnknownClientStateType(any.type_url).into()),
}
}
}

impl ClientState for AnyClientState {
fn chain_id(&self) -> String {
todo!()
Expand Down
12 changes: 12 additions & 0 deletions modules/src/ics02_client/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ pub enum Kind {
#[error("header verification failed")]
HeaderVerificationFailure,

#[error("unknown client state type: {0}")]
UnknownClientStateType(String),

#[error("invalid raw client state")]
InvalidRawClientState,

#[error("invalid raw header")]
InvalidRawHeader,

#[error("Protobuf decoding failure")]
ProtoDecodingFailure,

#[error("mismatch between client and arguments types, expected: {0:?}")]
ClientArgsTypeMismatch(ClientType),
}
Expand Down
10 changes: 6 additions & 4 deletions modules/src/ics03_connection/connection.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use crate::ics03_connection::error::{Error, Kind};
use crate::ics23_commitment::commitment::CommitmentPrefix;
use crate::ics24_host::error::ValidationError;
use crate::ics24_host::identifier::{ClientId, ConnectionId};
use crate::try_from_raw::TryFromRaw;
use serde_derive::{Deserialize, Serialize};

// Import proto declarations.
use crate::ics24_host::error::ValidationError;
use ibc_proto::connection::{ConnectionEnd as RawConnectionEnd, Counterparty as RawCounterparty};
use ibc_proto::ibc::connection::{
ConnectionEnd as RawConnectionEnd, Counterparty as RawCounterparty,
};

use serde_derive::{Deserialize, Serialize};
use std::convert::{TryFrom, TryInto};

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
Expand Down
2 changes: 1 addition & 1 deletion modules/src/ics03_connection/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ where
result,
log,
events,
} = conn_open_try::process(ctx, msg)?;
} = conn_open_try::process(ctx, *msg)?;

keep(ctx, result.clone())?;
Ok(HandlerOutput::builder()
Expand Down
6 changes: 3 additions & 3 deletions modules/src/ics03_connection/handler/conn_open_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,20 @@ mod tests {
ctx: default_context
.clone()
.with_client_state(dummy_msg.client_id(), 10),
msg: ConnectionMsg::ConnectionOpenTry(dummy_msg.clone()),
msg: ConnectionMsg::ConnectionOpenTry(Box::new(dummy_msg.clone())),
want_pass: true,
},
Test {
name: "Processing fails because no client exists".to_string(),
ctx: default_context.clone(),
msg: ConnectionMsg::ConnectionOpenTry(dummy_msg.clone()),
msg: ConnectionMsg::ConnectionOpenTry(Box::new(dummy_msg.clone())),
want_pass: false,
},
Test {
name: "Processing fails because connection exists in the store already".to_string(),
ctx: default_context
.add_connection(dummy_msg.connection_id().clone(), try_conn_end.clone()),
msg: ConnectionMsg::ConnectionOpenTry(dummy_msg.clone()),
msg: ConnectionMsg::ConnectionOpenTry(Box::new(dummy_msg.clone())),
want_pass: false,
},
]
Expand Down
81 changes: 39 additions & 42 deletions modules/src/ics03_connection/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,26 @@
//! TODO: Separate the Cosmos-SDK specific functionality from canonical ICS types. Decorators?

#![allow(clippy::too_many_arguments)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code, I think it's safe to remove this now.


use crate::ics02_client::client_def::AnyClientState;
use crate::ics03_connection::connection::{validate_version, validate_versions, Counterparty};
use crate::ics03_connection::error::{Error, Kind};
use crate::ics24_host::identifier::{ClientId, ConnectionId};
use crate::proofs::{ConsensusProof, Proofs};
use crate::try_from_raw::TryFromRaw;
use crate::tx_msg::Msg;
use ibc_proto::connection::MsgConnectionOpenAck as RawMsgConnectionOpenAck;
use ibc_proto::connection::MsgConnectionOpenConfirm as RawMsgConnectionOpenConfirm;
use ibc_proto::connection::MsgConnectionOpenInit as RawMsgConnectionOpenInit;
use ibc_proto::connection::MsgConnectionOpenTry as RawMsgConnectionOpenTry;

use crate::ics02_client::client_def::AnyClientState;
use ibc_proto::ibc::connection::MsgConnectionOpenAck as RawMsgConnectionOpenAck;
use ibc_proto::ibc::connection::MsgConnectionOpenConfirm as RawMsgConnectionOpenConfirm;
use ibc_proto::ibc::connection::MsgConnectionOpenInit as RawMsgConnectionOpenInit;
use ibc_proto::ibc::connection::MsgConnectionOpenTry as RawMsgConnectionOpenTry;

use tendermint::account::Id as AccountId;
use tendermint::block::Height;

use serde_derive::{Deserialize, Serialize};
use std::convert::TryInto;
use std::str::{from_utf8, FromStr};
use tendermint::account::Id as AccountId;
use tendermint::block::Height;

/// Message type for the `MsgConnectionOpenInit` message.
pub const TYPE_MSG_CONNECTION_OPEN_INIT: &str = "connection_open_init";
Expand All @@ -48,7 +51,7 @@ pub const TYPE_MSG_CONNECTION_OPEN_CONFIRM: &str = "connection_open_confirm";
#[derive(Clone, Debug)]
pub enum ConnectionMsg {
ConnectionOpenInit(MsgConnectionOpenInit),
ConnectionOpenTry(MsgConnectionOpenTry),
ConnectionOpenTry(Box<MsgConnectionOpenTry>),
// ConnectionOpenAck(MsgConnectionOpenAck),
// ConnectionOpenConfirm(MsgConnectionOpenConfirm),
}
Expand Down Expand Up @@ -214,31 +217,19 @@ impl Msg for MsgConnectionOpenTry {
}
}

pub fn unpack_client_state(
any_client_raw: ::std::option::Option<::prost_types::Any>,
) -> Result<Option<AnyClientState>, anomaly::Error<Kind>> {
match any_client_raw {
None => Ok(None),
Some(_client_raw) => {
// TODO deserialize
//let _cs = client_raw.value.into();
Ok(None)
}
}
}

impl TryFromRaw for MsgConnectionOpenTry {
type Error = Error;
type RawType = RawMsgConnectionOpenTry;
type Error = anomaly::Error<Kind>;

fn try_from(msg: RawMsgConnectionOpenTry) -> Result<Self, Self::Error> {
let proof_height = msg
.proof_height
.ok_or_else(|| Kind::MissingProofHeight)?
.epoch_height;
.epoch_height; // FIXME: This is wrong as it does not take the epoch number into account
let consensus_height = msg
.consensus_height
.ok_or_else(|| Kind::MissingConsensusHeight)?
.epoch_height;
.epoch_height; // FIXME: This is wrong as it does not take the epoch number into account
let consensus_proof_obj = ConsensusProof::new(msg.proof_consensus.into(), consensus_height)
.map_err(|e| Kind::InvalidProof.context(e))?;

Expand All @@ -256,7 +247,10 @@ impl TryFromRaw for MsgConnectionOpenTry {
.client_id
.parse()
.map_err(|e| Kind::IdentifierError.context(e))?,
client_state: unpack_client_state(msg.client_state)
client_state: msg
.client_state
.map(AnyClientState::from_any)
.transpose()
.map_err(|e| Kind::InvalidProof.context(e))?,
counterparty: msg
.counterparty
Expand Down Expand Up @@ -354,11 +348,11 @@ impl TryFromRaw for MsgConnectionOpenAck {
let proof_height = msg
.proof_height
.ok_or_else(|| Kind::MissingProofHeight)?
.epoch_height;
.epoch_height; // FIXME: This is wrong as it does not take the epoch number into account
let consensus_height = msg
.consensus_height
.ok_or_else(|| Kind::MissingConsensusHeight)?
.epoch_height;
.epoch_height; // FIXME: This is wrong as it does not take the epoch number into account
let consensus_proof_obj = ConsensusProof::new(msg.proof_consensus.into(), consensus_height)
.map_err(|e| Kind::InvalidProof.context(e))?;

Expand All @@ -372,7 +366,10 @@ impl TryFromRaw for MsgConnectionOpenAck {
.connection_id
.parse()
.map_err(|e| Kind::IdentifierError.context(e))?,
client_state: unpack_client_state(msg.client_state)
client_state: msg
.client_state
.map(AnyClientState::from_any)
.transpose()
.map_err(|e| Kind::InvalidProof.context(e))?,
version: validate_version(msg.version).map_err(|e| Kind::InvalidVersion.context(e))?,
proofs: Proofs::new(
Expand Down Expand Up @@ -444,7 +441,7 @@ impl TryFromRaw for MsgConnectionOpenConfirm {
let proof_height = msg
.proof_height
.ok_or_else(|| Kind::MissingProofHeight)?
.epoch_height;
.epoch_height; // FIXME: This is wrong as it does not take the epoch number into account
Ok(Self {
connection_id: msg
.connection_id
Expand All @@ -462,14 +459,14 @@ impl TryFromRaw for MsgConnectionOpenConfirm {

#[cfg(test)]
pub mod test_util {
use ibc_proto::connection::Counterparty as RawCounterparty;
use ibc_proto::connection::MsgConnectionOpenAck as RawMsgConnectionOpenAck;
use ibc_proto::connection::MsgConnectionOpenConfirm as RawMsgConnectionOpenConfirm;
use ibc_proto::connection::MsgConnectionOpenInit as RawMsgConnectionOpenInit;
use ibc_proto::connection::MsgConnectionOpenTry as RawMsgConnectionOpenTry;
use ibc_proto::ibc::connection::Counterparty as RawCounterparty;
use ibc_proto::ibc::connection::MsgConnectionOpenAck as RawMsgConnectionOpenAck;
use ibc_proto::ibc::connection::MsgConnectionOpenConfirm as RawMsgConnectionOpenConfirm;
use ibc_proto::ibc::connection::MsgConnectionOpenInit as RawMsgConnectionOpenInit;
use ibc_proto::ibc::connection::MsgConnectionOpenTry as RawMsgConnectionOpenTry;

use ibc_proto::client::Height;
use ibc_proto::commitment::MerklePrefix;
use ibc_proto::ibc::client::Height;
use ibc_proto::ibc::commitment::MerklePrefix;

pub fn get_dummy_proof() -> Vec<u8> {
"Y29uc2Vuc3VzU3RhdGUvaWJjb25lY2xpZW50LzIy"
Expand Down Expand Up @@ -573,12 +570,12 @@ mod tests {
MsgConnectionOpenAck, MsgConnectionOpenConfirm, MsgConnectionOpenTry,
};
use crate::try_from_raw::TryFromRaw;
use ibc_proto::client::Height;
use ibc_proto::connection::Counterparty as RawCounterparty;
use ibc_proto::connection::MsgConnectionOpenAck as RawMsgConnectionOpenAck;
use ibc_proto::connection::MsgConnectionOpenConfirm as RawMsgConnectionOpenConfirm;
use ibc_proto::connection::MsgConnectionOpenInit as RawMsgConnectionOpenInit;
use ibc_proto::connection::MsgConnectionOpenTry as RawMsgConnectionOpenTry;
use ibc_proto::ibc::client::Height;
use ibc_proto::ibc::connection::Counterparty as RawCounterparty;
use ibc_proto::ibc::connection::MsgConnectionOpenAck as RawMsgConnectionOpenAck;
use ibc_proto::ibc::connection::MsgConnectionOpenConfirm as RawMsgConnectionOpenConfirm;
use ibc_proto::ibc::connection::MsgConnectionOpenInit as RawMsgConnectionOpenInit;
use ibc_proto::ibc::connection::MsgConnectionOpenTry as RawMsgConnectionOpenTry;

#[test]
fn parse_connection_open_init_msg() {
Expand Down
14 changes: 9 additions & 5 deletions modules/src/ics04_channel/channel.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use crate::ics04_channel::error::{self, Error, Kind};
use crate::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
use crate::try_from_raw::TryFromRaw;

use ibc_proto::ibc::channel::Channel as RawChannel;

use anomaly::fail;
use core::str::FromStr;
use ibc_proto::channel::Channel as RawChannel;
use serde_derive::{Deserialize, Serialize};
use std::str::FromStr;

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct ChannelEnd {
Expand Down Expand Up @@ -221,11 +223,13 @@ impl State {

#[cfg(test)]
mod tests {
use std::str::FromStr;

use crate::ics04_channel::channel::ChannelEnd;
use crate::try_from_raw::TryFromRaw;
use core::str::FromStr;
use ibc_proto::channel::Channel as RawChannel;
use ibc_proto::channel::Counterparty as RawCounterparty;

use ibc_proto::ibc::channel::Channel as RawChannel;
use ibc_proto::ibc::channel::Counterparty as RawCounterparty;

#[test]
fn channel_end_try_from_raw() {
Expand Down
Loading