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

Fix for client query subcommands #235

Merged
merged 6 commits into from
Sep 21, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 49 additions & 13 deletions modules/src/ics02_client/client_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ 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::ics07_tendermint::consensus_state::ConsensusState as TendermintConsensusState;
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 ibc_proto::ibc::tendermint::{
ClientState as RawTendermintClientState, ConsensusState as RawTendermintConsensusState,
};

use ::tendermint::block::Height;

Expand Down Expand Up @@ -119,11 +122,14 @@ pub enum AnyClientState {
Mock(MockClientState),
}

impl AnyClientState {
pub fn from_any(any: prost_types::Any) -> Result<Self, Error> {
match any.type_url.as_str() {
impl TryFromRaw for AnyClientState {
type RawType = prost_types::Any;
type Error = Error;

fn try_from(value: Self::RawType) -> Result<Self, Self::Error> {
match value.type_url.as_str() {
"ibc.tendermint.ClientState" => {
adizere marked this conversation as resolved.
Show resolved Hide resolved
let raw = RawTendermintClientState::decode(any.value.as_ref())
let raw = RawTendermintClientState::decode(value.value.as_ref())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious on why you changed from_any that transforms from a prost type to the try_from for TryFromRaw impl. I liked the old version a bit better.
Also value.value.as_ref() doesn't ready very well :)

Copy link
Member Author

@adizere adizere Sep 18, 2020

Choose a reason for hiding this comment

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

It was necessary for AnyClientState to implement the TryFromRaw trait, otherwise the call below would fail:

https://github.com/informalsystems/ibc-rs/blob/106c459cab6cc1a6359759c7bfc2d0d62e004686/relayer-cli/src/commands/query/client.rs#L80-L81

The failure would occur because the function chain.query::<T> requires the trait bound: T: TryFromRaw.

Also value.value.as_ref() doesn't ready very well :)

Agree! Didn't notice that, will fix.

.map_err(|e| error::Kind::ProtoDecodingFailure.context(e))?;
let client_state = TendermintClientState::try_from(raw)
.map_err(|e| error::Kind::InvalidRawClientState.context(e))?;
Expand All @@ -133,15 +139,15 @@ impl AnyClientState {

#[cfg(test)]
"ibc.mock.ClientState" => {
let raw = RawMockClientState::decode(any.value.as_ref())
let raw = RawMockClientState::decode(value.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()),
_ => Err(error::Kind::UnknownClientStateType(value.type_url).into()),
}
}
}
Expand All @@ -160,21 +166,21 @@ impl ClientState for AnyClientState {
}
}

fn is_frozen(&self) -> bool {
fn latest_height(&self) -> Height {
match self {
AnyClientState::Tendermint(tm_state) => tm_state.is_frozen(),
Self::Tendermint(tm_state) => tm_state.latest_height(),

#[cfg(test)]
AnyClientState::Mock(mock_state) => mock_state.is_frozen(),
Self::Mock(mock_state) => mock_state.latest_height(),
}
}

fn latest_height(&self) -> Height {
fn is_frozen(&self) -> bool {
match self {
Self::Tendermint(tm_state) => tm_state.latest_height(),
AnyClientState::Tendermint(tm_state) => tm_state.is_frozen(),

#[cfg(test)]
Self::Mock(mock_state) => mock_state.latest_height(),
AnyClientState::Mock(mock_state) => mock_state.is_frozen(),
}
}
}
Expand All @@ -187,6 +193,36 @@ pub enum AnyConsensusState {
Mock(MockConsensusState),
}

impl TryFromRaw for AnyConsensusState {
type RawType = prost_types::Any;
type Error = Error;

fn try_from(value: Self::RawType) -> Result<Self, Self::Error> {
match value.type_url.as_str() {
"ibc.tendermint.ConsensusState" => {
let raw = RawTendermintConsensusState::decode(value.value.as_ref())
.map_err(|e| error::Kind::ProtoDecodingFailure.context(e))?;
let consensus_state = TendermintConsensusState::try_from(raw)
.map_err(|e| error::Kind::InvalidRawConsensusState.context(e))?;

Ok(AnyConsensusState::Tendermint(consensus_state))
}

// TODO get this to compile! -- Add the ClientConsensusState definition in ibc-proto.
// #[cfg(test)]
// "ibc.mock.ConsensusState" => {
// let raw = RawMockConsensusState::decode(value.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::UnknownConsensusStateType(value.type_url).into()),
}
}
}

impl ConsensusState for AnyConsensusState {
fn client_type(&self) -> ClientType {
todo!()
Expand Down
6 changes: 6 additions & 0 deletions modules/src/ics02_client/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,15 @@ pub enum Kind {
#[error("unknown client state type: {0}")]
UnknownClientStateType(String),

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

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

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

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

Expand Down
6 changes: 3 additions & 3 deletions modules/src/ics03_connection/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ impl Msg for MsgConnectionOpenTry {
}

impl TryFromRaw for MsgConnectionOpenTry {
type Error = Error;
type RawType = RawMsgConnectionOpenTry;
type Error = Error;

fn try_from(msg: RawMsgConnectionOpenTry) -> Result<Self, Self::Error> {
let proof_height = msg
Expand Down Expand Up @@ -249,7 +249,7 @@ impl TryFromRaw for MsgConnectionOpenTry {
.map_err(|e| Kind::IdentifierError.context(e))?,
client_state: msg
.client_state
.map(AnyClientState::from_any)
.map(AnyClientState::try_from)
.transpose()
.map_err(|e| Kind::InvalidProof.context(e))?,
counterparty: msg
Expand Down Expand Up @@ -368,7 +368,7 @@ impl TryFromRaw for MsgConnectionOpenAck {
.map_err(|e| Kind::IdentifierError.context(e))?,
client_state: msg
.client_state
.map(AnyClientState::from_any)
.map(AnyClientState::try_from)
.transpose()
.map_err(|e| Kind::InvalidProof.context(e))?,
version: validate_version(msg.version).map_err(|e| Kind::InvalidVersion.context(e))?,
Expand Down
2 changes: 1 addition & 1 deletion modules/src/ics07_tendermint/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ impl crate::ics02_client::state::ClientState for ClientState {
}

impl TryFromRaw for ClientState {
type Error = Error;
type RawType = ibc_proto::ibc::tendermint::ClientState;
type Error = Error;

fn try_from(raw: Self::RawType) -> Result<Self, Self::Error> {
Ok(Self {
Expand Down
11 changes: 11 additions & 0 deletions modules/src/ics07_tendermint/consensus_state.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::ics02_client::client_type::ClientType;
use crate::ics23_commitment::commitment::CommitmentRoot;

use crate::ics07_tendermint::error::Error;
use crate::try_from_raw::TryFromRaw;
use serde_derive::{Deserialize, Serialize};
use tendermint::Hash;

Expand All @@ -12,6 +14,15 @@ pub struct ConsensusState {
pub next_validators_hash: Hash,
}

impl TryFromRaw for ConsensusState {
type RawType = ibc_proto::ibc::tendermint::ConsensusState;
type Error = Error;

fn try_from(_raw: Self::RawType) -> Result<Self, Self::Error> {
unimplemented!()
adizere marked this conversation as resolved.
Show resolved Hide resolved
}
}

impl ConsensusState {
pub fn new(
root: CommitmentRoot,
Expand Down
4 changes: 2 additions & 2 deletions modules/src/ics24_host/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub const IBC_QUERY_PATH: &str = "store/ibc/key";
pub enum Path {
ClientType(ClientId),
ClientState(ClientId),
adizere marked this conversation as resolved.
Show resolved Hide resolved
ConsensusState(ClientId, u64),
ClientConsensusState(ClientId, u64),
ClientConnections(ClientId),
Connections(ConnectionId),
Ports(PortId),
Expand Down Expand Up @@ -47,7 +47,7 @@ impl Display for Path {
match &self {
Path::ClientType(id) => write!(f, "clients/{}/clientType", id),
Path::ClientState(id) => write!(f, "clients/{}/clientState", id),
Path::ConsensusState(id, height) => {
Path::ClientConsensusState(id, height) => {
write!(f, "clients/{}/consensusState/{}", id, height)
}
Path::ClientConnections(id) => write!(f, "clients/{}/connections", id),
Expand Down
2 changes: 1 addition & 1 deletion modules/src/mock_client/client_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl ClientDef for MockClient {
_expected_consensus_state: &AnyConsensusState,
) -> Result<(), Box<dyn std::error::Error>> {
let client_prefixed_path =
Path::ConsensusState(client_id.clone(), height.value()).to_string();
Path::ClientConsensusState(client_id.clone(), height.value()).to_string();

let _path = apply_prefix(prefix, client_prefixed_path)?;

Expand Down
2 changes: 1 addition & 1 deletion modules/src/mock_client/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ impl From<MockClientState> for AnyClientState {
}

impl TryFromRaw for MockClientState {
type Error = Error;
type RawType = ibc_proto::ibc::mock::ClientState;
type Error = Error;

fn try_from(raw: Self::RawType) -> Result<Self, Self::Error> {
let raw_header = raw
Expand Down
1 change: 0 additions & 1 deletion relayer-cli/src/commands/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ pub enum QueryClientCmds {
/// The `query client state` subcommand
#[options(help = "query client full state")]
State(client::QueryClientStateCmd),
/// The `query client consensus` subcommand

/// The `query client consensus` subcommand
#[options(help = "query client consensus")]
Expand Down
72 changes: 27 additions & 45 deletions relayer-cli/src/commands/query/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ use crate::prelude::*;
use abscissa_core::{Command, Options, Runnable};
use relayer::config::{ChainConfig, Config};

use ibc::ics02_client::client_def::{AnyClientState, AnyConsensusState};
use ibc::ics24_host::error::ValidationError;
use ibc::ics24_host::identifier::{ClientId, ConnectionId};
use ibc::ics24_host::Path::ClientConnections;
use ibc::ics24_host::Path::{ClientConnections, ClientConsensusState, ClientState};
use relayer::chain::Chain;
use relayer::chain::CosmosSDKChain;
use tendermint::chain::Id as ChainId;
Expand All @@ -19,7 +20,7 @@ pub struct QueryClientStateCmd {
#[options(free, help = "identifier of the client to query")]
client_id: Option<String>,

#[options(help = "height of the state to query", short = "h")]
#[options(help = "the chain height which this query should reflect", short = "h")]
height: Option<u64>,

#[options(help = "whether proof is required", short = "p")]
Expand Down Expand Up @@ -56,6 +57,12 @@ impl QueryClientStateCmd {
}
}

/// Command for handling a query for a client's state.
/// To run with proof:
/// cargo run --bin relayer -- -c relayer/tests/config/fixtures/simple_config.toml query client state ibc-test ethbridge --height 3
///
/// Run without proof:
/// cargo run --bin relayer -- -c relayer/tests/config/fixtures/simple_config.toml query client state ibc-test ethbridge --height 3 -p false
impl Runnable for QueryClientStateCmd {
fn run(&self) {
let config = app_config();
Expand All @@ -69,27 +76,13 @@ impl Runnable for QueryClientStateCmd {
};
status_info!("Options", "{:?}", opts);

// run with proof:
// cargo run --bin relayer -- -c relayer/tests/config/fixtures/simple_config.toml query client state ibc-test ethbridge --height 3
//
// run without proof:
// cargo run --bin relayer -- -c relayer/tests/config/fixtures/simple_config.toml query client state ibc-test ethbridge --height 3 -p false
//
// Note: currently both fail in amino_unmarshal_binary_length_prefixed().
// To test this start a Gaia node and configure a client using the go relayer.
let _chain = CosmosSDKChain::from_config(chain_config).unwrap();
/* Todo: Implement client full state query
let res = block_on(query_client_full_state(
&chain,
opts.height,
opts.client_id.clone(),
opts.proof,
));
let chain = CosmosSDKChain::from_config(chain_config).unwrap();
let res =
chain.query::<AnyClientState>(ClientState(opts.client_id), opts.height, opts.proof);
match res {
Ok(cs) => status_info!("client state query result: ", "{:?}", cs.client_state),
Ok(cs) => status_info!("client state query result: ", "{:?}", cs),
Err(e) => status_info!("client state query error: ", "{:?}", e),
}
*/
}
}

Expand All @@ -102,10 +95,10 @@ pub struct QueryClientConsensusCmd {
#[options(free, help = "identifier of the client to query")]
client_id: Option<String>,

#[options(free, help = "height of the consensus state to query")]
#[options(free, help = "height of the client's consensus state to query")]
consensus_height: Option<u64>,

#[options(help = "height of the consensus state to query", short = "h")]
#[options(help = "the chain height which this query should reflect", short = "h")]
height: Option<u64>,

#[options(help = "whether proof is required", short = "p")]
Expand Down Expand Up @@ -149,6 +142,12 @@ impl QueryClientConsensusCmd {
}
}

/// Implementation of the query for a client's consensus state at a certain height.
/// Run with proof:
/// cargo run --bin relayer -- -c simple_config.toml query client consensus ibc0 ibconeclient 22
///
/// Run without proof:
/// cargo run --bin relayer -- -c simple_config.toml query client consensus ibc0 ibconeclient 22 -p false
impl Runnable for QueryClientConsensusCmd {
fn run(&self) {
let config = app_config();
Expand All @@ -162,33 +161,16 @@ impl Runnable for QueryClientConsensusCmd {
};
status_info!("Options", "{:?}", opts);

// run with proof:
// cargo run --bin relayer -- -c simple_config.toml query client consensus ibc0 ibconeclient 22
//
// run without proof:
// cargo run --bin relayer -- -c simple_config.toml query client consensus ibc0 ibconeclient 22 -p false
//
// Note: currently both fail in amino_unmarshal_binary_length_prefixed().
// To test this start a Gaia node and configure a client using the go relayer.
let _chain = CosmosSDKChain::from_config(chain_config).unwrap();
/* Todo: Implement client consensus state query
let res = block_on(query_client_consensus_state(
&chain,
let chain = CosmosSDKChain::from_config(chain_config).unwrap();
let res = chain.query::<AnyConsensusState>(
ClientConsensusState(opts.client_id, opts.consensus_height),
opts.height,
opts.client_id,
opts.consensus_height,
opts.proof,
));
);
match res {
Ok(cs) => status_info!(
"client consensus state query result: ",
"{:?}",
cs.consensus_state
),
Ok(cs) => status_info!("client consensus state query result: ", "{:?}", cs),
Err(e) => status_info!("client consensus state query error: ", "{:?}", e),
}

*/
}
}

Expand Down Expand Up @@ -222,7 +204,7 @@ pub struct QueryClientConnectionsCmd {
#[options(free, help = "identifier of the client to query")]
client_id: Option<String>,

#[options(help = "height of the state to query", short = "h")]
#[options(help = "the chain height which this query should reflect", short = "h")]
height: Option<u64>,

#[options(help = "whether proof is required", short = "p")]
Expand Down
Loading