Skip to content

Commit

Permalink
Removed TryFromRaw, implemented DomainType where needed (informalsyst…
Browse files Browse the repository at this point in the history
…ems#252)

* Removed TryFromRaw, implemented DomainType where needed

* Update modules/src/ics03_connection/connection.rs

Co-authored-by: Adi Seredinschi <adi@informal.systems>

Co-authored-by: Adi Seredinschi <adi@informal.systems>
  • Loading branch information
greg-szabo and adizere committed Sep 24, 2020
1 parent 38a0e1c commit 4224310
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 93 deletions.
14 changes: 11 additions & 3 deletions docs/architecture/adr-001-repo.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,25 @@ name prefixed with "Raw", for example:
use ibc_proto::channel::Channel as RawChannel;
```

For any Raw data type that is defined in `ibc-proto` we implement the `TryFromRaw` trait, which serves as a translation
For any Raw data type that is defined in `ibc-proto` we implement the `DomainType` trait, which serves as a translation
& validation layer between the proto ("Raw") types and the domain types. For example, for a `Channel` we do as follows:

```Rust
impl TryFromRaw for ChannelEnd {
type RawType = RawChannel;
impl DomainType<RawChannel> for ChannelEnd {}

impl TryFrom<RawChannel> for ChannelEnd {
type Error = anomaly::Error<Kind>;

fn try_from(value: RawChannel) -> Result<Self, Self::Error> {
// Translate, validate each field from RawChannel into a Channel.
}
}

impl From<ChannelEnd> for RawChannel {
fn from(value: ChannelEnd) -> Self {
// Translate Channel into a RawChannel
}
}
```

This issue [#130](https://github.com/informalsystems/ibc-rs/issues/130) is a good starting place for more context
Expand Down
15 changes: 10 additions & 5 deletions relayer-cli/src/commands/query/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ use ibc::ics04_channel::channel::ChannelEnd;
use ibc::ics24_host::identifier::{ChannelId, PortId};
use ibc::ics24_host::Path::ChannelEnds;

use crate::error::{Error, Kind};
use ibc::ics24_host::error::ValidationError;
use relayer::chain::{Chain, CosmosSDKChain};
use tendermint::chain::Id as ChainId;
use tendermint_proto::DomainType;

#[derive(Clone, Command, Debug, Options)]
pub struct QueryChannelEndCmd {
Expand Down Expand Up @@ -97,11 +99,14 @@ impl Runnable for QueryChannelEndCmd {
// run without proof:
// cargo run --bin relayer -- -c relayer/tests/config/fixtures/simple_config.toml query channel end ibc-test firstport firstchannel --height 3 -p false
let chain = CosmosSDKChain::from_config(chain_config).unwrap();
let res = chain.query::<ChannelEnd>(
ChannelEnds(opts.port_id, opts.channel_id),
opts.height,
opts.proof,
);
let res: Result<ChannelEnd, Error> = chain
.query(
ChannelEnds(opts.port_id, opts.channel_id),
opts.height,
opts.proof,
)
.map_err(|e| Kind::Query.context(e).into())
.and_then(|v| ChannelEnd::decode_vec(&v).map_err(|e| Kind::Query.context(e).into()));

match res {
Ok(cs) => status_info!("Result for channel end query: ", "{:?}", cs),
Expand Down
16 changes: 8 additions & 8 deletions relayer-cli/src/commands/query/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use relayer::config::{ChainConfig, Config};

use crate::error::{Error, Kind};
use ibc::ics02_client::client_def::{AnyClientState, AnyConsensusState};
use ibc::ics02_client::raw::ConnectionIds as ConnectionIDs;
use ibc::ics24_host::error::ValidationError;
use ibc::ics24_host::identifier::{ClientId, ConnectionId};
use ibc::ics24_host::identifier::ClientId;
use ibc::ics24_host::Path::{ClientConnections, ClientConsensusState, ClientState};
use relayer::chain::Chain;
use relayer::chain::CosmosSDKChain;
Expand Down Expand Up @@ -81,7 +82,7 @@ impl Runnable for QueryClientStateCmd {
let chain = CosmosSDKChain::from_config(chain_config).unwrap();

let res: Result<AnyClientState, Error> = chain
.abci_query(ClientState(opts.client_id), opts.height, opts.proof)
.query(ClientState(opts.client_id), opts.height, opts.proof)
.map_err(|e| Kind::Query.context(e).into())
.and_then(|v| {
AnyClientState::decode_vec(&v).map_err(|e| Kind::Query.context(e).into())
Expand Down Expand Up @@ -170,7 +171,7 @@ impl Runnable for QueryClientConsensusCmd {

let chain = CosmosSDKChain::from_config(chain_config).unwrap();
let res: Result<AnyConsensusState, Error> = chain
.abci_query(
.query(
ClientConsensusState(opts.client_id, opts.consensus_height),
opts.height,
opts.proof,
Expand Down Expand Up @@ -284,11 +285,10 @@ impl Runnable for QueryClientConnectionsCmd {
status_info!("Options", "{:?}", opts);

let chain = CosmosSDKChain::from_config(chain_config).unwrap();
let res = chain.query::<Vec<ConnectionId>>(
ClientConnections(opts.client_id),
opts.height,
opts.proof,
);
let res: Result<ConnectionIDs, Error> = chain
.query(ClientConnections(opts.client_id), opts.height, opts.proof)
.map_err(|e| Kind::Query.context(e).into())
.and_then(|v| ConnectionIDs::decode_vec(&v).map_err(|e| Kind::Query.context(e).into()));
match res {
Ok(cs) => status_info!("client connections query result: ", "{:?}", cs),
Err(e) => status_info!("client connections query error", "{}", e),
Expand Down
8 changes: 6 additions & 2 deletions relayer-cli/src/commands/query/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ use crate::prelude::*;
use abscissa_core::{Command, Options, Runnable};
use relayer::config::{ChainConfig, Config};

use crate::error::{Error, Kind};
use ibc::ics24_host::error::ValidationError;
use ibc::ics24_host::identifier::ConnectionId;
use ibc::ics24_host::Path::Connections;
use relayer::chain::{Chain, CosmosSDKChain};
use tendermint::chain::Id as ChainId;
use tendermint_proto::DomainType;

use ibc::ics03_connection::connection::ConnectionEnd;

Expand Down Expand Up @@ -85,8 +87,10 @@ impl Runnable for QueryConnectionEndCmd {
let chain = CosmosSDKChain::from_config(chain_config).unwrap();
// run without proof:
// cargo run --bin relayer -- -c relayer/tests/config/fixtures/simple_config.toml query connection end ibc-test connectionidone --height 3 -p false
let res =
chain.query::<ConnectionEnd>(Connections(opts.connection_id), opts.height, opts.proof);
let res: Result<ConnectionEnd, Error> = chain
.query(Connections(opts.connection_id), opts.height, opts.proof)
.map_err(|e| Kind::Query.context(e).into())
.and_then(|v| ConnectionEnd::decode_vec(&v).map_err(|e| Kind::Query.context(e).into()));

match res {
Ok(cs) => status_info!("connection query result: ", "{:?}", cs),
Expand Down
69 changes: 40 additions & 29 deletions relayer-cli/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@
unused_qualifications
)]

use ibc::ics03_connection::connection::ConnectionEnd;
use ibc::ics03_connection::connection::State as ConnectionState;
use ibc::ics02_client::raw::ConnectionIds as DomainTypeClientConnections;
use ibc::ics04_channel::channel::{ChannelEnd, Order, State as ChannelState};
use ibc::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
use ibc::ics24_host::Path::{ChannelEnds, ClientConnections, Connections};
use ibc::ics24_host::Path::{ChannelEnds, ClientConnections};
use relayer::chain::{Chain, CosmosSDKChain};
use relayer::config::{ChainConfig, Config};
use std::str::FromStr;
use tendermint::chain::Id;
use tendermint::net::Address;
use tendermint_proto::DomainType;

/// Configuration that connects to the informaldev/simd DockerHub image running on localhost.
fn simd_config() -> Config {
Expand All @@ -47,14 +47,18 @@ fn simd_chain() -> CosmosSDKChain {
#[test]
#[ignore]
fn query_connection_id() {
/* the current informaldev/simd image has an incompatible (old?) protobuf implementation
let chain = simd_chain();
let query = chain
.query::<ConnectionEnd>(
Connections(ConnectionId::from_str("connectionidone").unwrap()),
0,
false,
)
.unwrap();
let query = ConnectionEnd::decode_vec(
&chain
.abci_query(
Connections(ConnectionId::from_str("connectionidone").unwrap()),
0,
false,
)
.unwrap(),
);
.unwrap();
assert_eq!(query.state(), &ConnectionState::Init);
assert_eq!(query.client_id(), "clientidone");
Expand All @@ -65,23 +69,27 @@ fn query_connection_id() {
query.versions(),
vec!["(1,[ORDER_ORDERED,ORDER_UNORDERED])"]
);
*/
}

/// Query channel ID. Requires the informaldev/simd Docker image running on localhost.
#[test]
#[ignore]
fn query_channel_id() {
let chain = simd_chain();
let query = chain
.query::<ChannelEnd>(
ChannelEnds(
PortId::from_str("firstport").unwrap(),
ChannelId::from_str("firstchannel").unwrap(),
),
0,
false,
)
.unwrap();
let query = ChannelEnd::decode_vec(
&chain
.query(
ChannelEnds(
PortId::from_str("firstport").unwrap(),
ChannelId::from_str("firstchannel").unwrap(),
),
0,
false,
)
.unwrap(),
)
.unwrap();

assert_eq!(query.state(), &ChannelState::Init);
assert_eq!(query.ordering(), &Order::Ordered);
Expand All @@ -96,16 +104,19 @@ fn query_channel_id() {
#[ignore]
fn query_client_id() {
let chain = simd_chain();
let query = chain
.query::<Vec<ConnectionId>>(
ClientConnections(ClientId::from_str("clientidone").unwrap()),
0,
false,
)
.unwrap();
let query = DomainTypeClientConnections::decode_vec(
&chain
.query(
ClientConnections(ClientId::from_str("clientidone").unwrap()),
0,
false,
)
.unwrap(),
)
.unwrap();

assert_eq!(
query[0],
ConnectionId::from_str("connections/connectionidone").unwrap()
query.0[0],
ConnectionId::from_str("connectionidone").unwrap()
);
}
18 changes: 2 additions & 16 deletions relayer/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use ::tendermint_rpc::Client as RpcClient;

use ibc::ics02_client::state::{ClientState, ConsensusState};
use ibc::ics24_host::Path;
use ibc::try_from_raw::TryFromRaw;

use crate::config::ChainConfig;
use crate::error;
Expand Down Expand Up @@ -42,21 +41,8 @@ pub trait Chain {
/// Error types defined by this chain
type Error: Into<Box<dyn Error + Send + Sync + 'static>>;

/// Perform a generic `query`, and return the corresponding deserialized response data.
// This is going to be a blocking request.
// From the "Asynchronous Programming in Rust" book:
// Important extensions like `async fn` syntax in trait methods are still unimplemented
// https://rust-lang.github.io/async-book/01_getting_started/03_state_of_async_rust.html
// DEPRECATED: implement abci_query instead. Since this will be removed before the next release
// I'm commenting out the deprectaed warning. It would just confuse the clippy check in CI.
//#[deprecated(since = "0.0.4", note = "please use `abci_query` instead")]
fn query<T>(&self, data: Path, height: u64, prove: bool) -> Result<T, Self::Error>
where
T: TryFromRaw;

/// Perform a generic `query` using ABCI, and return the corresponding response data.
/// The naming is foreshadowing for an upcoming `grpc_query` function in the future.
fn abci_query(&self, data: Path, height: u64, prove: bool) -> Result<Vec<u8>, Self::Error>;
/// Perform a generic `query`, and return the corresponding response data.
fn query(&self, data: Path, height: u64, prove: bool) -> Result<Vec<u8>, Self::Error>;

/// Returns the chain's identifier
fn id(&self) -> &ChainId {
Expand Down
31 changes: 1 addition & 30 deletions relayer/src/chain/cosmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,12 @@ use core::future::Future;
use ibc::ics07_tendermint::client_state::ClientState;
use ibc::ics07_tendermint::consensus_state::ConsensusState;
use ibc::ics24_host::{Path, IBC_QUERY_PATH};
use ibc::try_from_raw::TryFromRaw;

use crate::client::rpc_requester::RpcRequester;
use crate::config::ChainConfig;
use crate::error::{Error, Kind};

use super::Chain;
use bytes::Bytes;
use prost::Message;
use std::str::FromStr;

pub struct CosmosSDKChain {
Expand Down Expand Up @@ -50,33 +47,7 @@ impl Chain for CosmosSDKChain {
type Requester = RpcRequester;
type Error = anomaly::Error<Kind>;

fn query<T>(&self, data: Path, height: u64, prove: bool) -> Result<T, Self::Error>
where
T: TryFromRaw,
{
let path = TendermintABCIPath::from_str(IBC_QUERY_PATH).unwrap();
if !data.is_provable() & prove {
return Err(Kind::Store
.context("requested proof for a path in the privateStore")
.into());
}
let response = block_on(abci_query(&self, path, data.to_string(), height, prove))?;

// Verify response proof, if requested.
if prove {
dbg!("Todo: implement proof verification."); // Todo: Verify proof
}

// Deserialize response data. This involves two steps: (1) `decode` from the wire bytes into
// a Prost type (such as `ibc_proto::channel::Channel`) called intuitively a raw type;
// and (2) then translate with `try_from` from the Prost raw type into an actual domain type
// (e.g., `ibc::ics04_channel::channel::ChannelEnd`).
T::RawType::decode(Bytes::from(response))
.map_err(|e| Kind::ResponseParsing.context(e).into())
.and_then(|r| T::try_from(r).map_err(|e| Kind::ResponseParsing.context(e).into()))
}

fn abci_query(&self, data: Path, height: u64, prove: bool) -> Result<Vec<u8>, Self::Error> {
fn query(&self, data: Path, height: u64, prove: bool) -> Result<Vec<u8>, Self::Error> {
let path = TendermintABCIPath::from_str(IBC_QUERY_PATH).unwrap();
if !data.is_provable() & prove {
return Err(Kind::Store
Expand Down

0 comments on commit 4224310

Please sign in to comment.