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

Implemented better Raw type handling #156

Merged
merged 3 commits into from
Jul 18, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 4 additions & 3 deletions modules/src/ics03_connection/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::exported::*;
use crate::ics03_connection::error::{Error, Kind};
use crate::ics23_commitment::CommitmentPrefix;
use crate::ics24_host::identifier::{ClientId, ConnectionId};
use crate::raw_decode::RawDecode;
use serde_derive::{Deserialize, Serialize};

// Import proto declarations.
Expand All @@ -17,10 +18,10 @@ pub struct ConnectionEnd {
versions: Vec<String>,
}

impl TryFrom<RawConnectionEnd> for ConnectionEnd {
impl RawDecode for ConnectionEnd {
type RawType = RawConnectionEnd;
type Error = anomaly::Error<Kind>;

fn try_from(value: RawConnectionEnd) -> Result<Self, Self::Error> {
fn validate(value: RawConnectionEnd) -> Result<Self, Self::Error> {
// Todo: Is validation complete here? (Code was moved from `from_proto_connection_end`.)
if value.id == "" {
return Err(Kind::ConnectionNotFound.into());
Expand Down
8 changes: 4 additions & 4 deletions modules/src/ics04_channel/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ use super::exported::*;
use crate::ics04_channel::error;
use crate::ics04_channel::error::{Error, Kind};
use crate::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
use crate::raw_decode::RawDecode;
use serde_derive::{Deserialize, Serialize};

// Import proto declarations.
use ibc_proto::channel::Channel as RawChannel;

use serde::export::TryFrom;
use std::str::FromStr;

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
Expand All @@ -19,10 +19,10 @@ pub struct ChannelEnd {
version: String,
}

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

fn try_from(value: RawChannel) -> Result<Self, Self::Error> {
fn validate(value: RawChannel) -> Result<Self, Self::Error> {
// Todo: Do validation of data here. This is just an example implementation for testing.
let ordering = match value.ordering {
0 => Order::None,
Expand Down
1 change: 1 addition & 0 deletions modules/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub mod ics24_host;
pub mod keys;
pub mod path;
pub mod proofs;
pub mod raw_decode;
pub mod tx_msg;

/// Height of a block, same as in `tendermint` crate
Expand Down
28 changes: 28 additions & 0 deletions modules/src/raw_decode.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//! RawDecode trait for automatic protobuf deserialization - currently implemented with prost
//! This is similar to the pattern of using the #[serde(from="RawType") derive for automatic
//! conversion with the TryFrom::try_from<InternalType>(value: RawType) trait for validation.
//! Only serde does this for JSON and here we need to do it with protobuf.
use bytes::Bytes;
use prost::{DecodeError, Message};
use std::convert::Into;
use std::default::Default;
use std::error::Error;
use std::marker::Sized;
use std::vec::Vec;

/// RawDecode trait needs to implement a validate() function and an Error type for the return of that function.
pub trait RawDecode: Sized {
/// RawType defines the prost-compiled protobuf-defined Rust struct
type RawType: Message + Default;

/// Error defines the error type returned from validation.
type Error: Into<Box<dyn Error + Send + Sync + 'static>>;

/// validate function will validate the incoming RawType and convert it to our internal type
fn validate(value: Self::RawType) -> Result<Self, Self::Error>;

/// raw_decode function will decode the type from RawType using Prost
fn raw_decode(wire: Vec<u8>) -> Result<Self::RawType, DecodeError> {
Self::RawType::decode(Bytes::from(wire))
}
}
2 changes: 0 additions & 2 deletions relayer/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ tokio = { version = "0.2.13", features = ["rt-util", "sync"] }
tracing = "0.1.13"
tracing-subscriber = "0.2.3"
futures = "0.3.5"
# If only we had a serde compatible protobuf library, we could get away from this dependency here.
ibc-proto = "0.1.0"

[dependencies.abscissa_core]
version = "0.5.2"
Expand Down
17 changes: 2 additions & 15 deletions relayer/cli/src/commands/query/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ use relayer::query::{query, Request};
use relayer_modules::ics04_channel::channel::ChannelEnd;
use relayer_modules::ics24_host::identifier::{ChannelId, PortId};

// Import protobuf definitions.
use ibc_proto::channel::Channel as RawChannel;

use crate::commands::utils::block_on;
use relayer::chain::tendermint::TendermintChain;
use relayer_modules::error::Error;
use relayer_modules::ics24_host::error::ValidationError;
use relayer_modules::path::{ChannelEndsPath, Path};
use std::str::FromStr;
Expand Down Expand Up @@ -112,21 +110,10 @@ impl Runnable for QueryChannelEndCmd {
};
status_info!("Options", "{:?}", opts);

// run with proof:
// cargo run --bin relayer -- -c relayer/relay/tests/config/fixtures/simple_config.toml query channel end ibc-test firstport firstchannel --height 3
//
// run without proof:
// cargo run --bin relayer -- -c relayer/relay/tests/config/fixtures/simple_config.toml query channel end ibc-test firstport firstchannel --height 3 -p false
//
// Note: currently both fail in amino_unmarshal_binary_length_prefixed().
// To test this start a Gaia node and configure a channel using the go relayer.
let chain = TendermintChain::from_config(chain_config).unwrap();
let res = block_on(query::<
TendermintChain,
RawChannel,
ChannelEnd,
QueryChannelOptions,
>(&chain, opts));
let res: Result<ChannelEnd, Error> = block_on(query(&chain, opts));

match res {
Ok(cs) => status_info!("connection query result: ", "{:?}", cs),
Expand Down
17 changes: 3 additions & 14 deletions relayer/cli/src/commands/query/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ use relayer::config::{ChainConfig, Config};
use crate::commands::utils::block_on;
use relayer::chain::tendermint::TendermintChain;
use relayer::query::{query, Request};
use relayer_modules::error::Error;
use relayer_modules::ics24_host::error::ValidationError;
use relayer_modules::ics24_host::identifier::ConnectionId;
use relayer_modules::path::{ConnectionPath, Path};
use tendermint::chain::Id as ChainId;

use ibc_proto::connection::ConnectionEnd as RawConnectionEnd;
use relayer_modules::ics03_connection::connection::ConnectionEnd;
use std::str::FromStr;
use tendermint::abci::Path as TendermintPath;
Expand Down Expand Up @@ -99,20 +99,9 @@ impl Runnable for QueryConnectionEndCmd {
status_info!("Options", "{:?}", opts);

let chain = TendermintChain::from_config(chain_config).unwrap();
// run with proof:
// cargo run --bin relayer -- -c relayer/relay/tests/config/fixtures/simple_config.toml query connection end ibc-test connectionidone --height 3
//
// run without proof:
// cargo run --bin relayer -- -c relayer/relay/tests/config/fixtures/simple_config.toml query connection end ibc-test connectionidone --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 res = block_on(query::<
TendermintChain,
RawConnectionEnd,
ConnectionEnd,
QueryConnectionOptions,
>(&chain, opts));
// cargo run --bin relayer -- -c relayer/relay/tests/config/fixtures/simple_config.toml query connection end ibc-test connectionidone --height 3 -p false
let res: Result<ConnectionEnd, Error> = block_on(query(&chain, opts));

match res {
Ok(cs) => status_info!("connection query result: ", "{:?}", cs),
Expand Down
2 changes: 0 additions & 2 deletions relayer/relay/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,5 @@ toml = "0.5"
tracing = "0.1.13"
tokio = "0.2"
serde_json = { version = "1" }
prost = "0.6.1"
bytes = "0.5.5"

[dev-dependencies]
31 changes: 12 additions & 19 deletions relayer/relay/src/query.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use crate::chain::Chain;
use bytes::Bytes;
use prost::Message;
use relayer_modules::error;
use std::convert::TryFrom;
use relayer_modules::raw_decode::RawDecode;
use tendermint::abci::Path as TendermintPath;
use tendermint::block;

Expand Down Expand Up @@ -32,20 +30,19 @@ fn is_query_store_with_proof(_path: &TendermintPath) -> bool {
}

/// Perform a generic `abci_query` on the given `chain`, and return the corresponding deserialized response data.
pub async fn query<C, R, T, O>(chain: &C, request: O) -> Result<T, error::Error>
pub async fn query<C, T, O>(chain: &C, request: O) -> Result<T, error::Error>
where
C: Chain, // Chain configuration
R: Message + Default, // Raw Struct type
T: TryFrom<R>, // Internal Struct type
O: Into<Request>, // Query Command configuration (opts)
C: Chain, // Chain configuration
T: RawDecode, // Internal Struct type (expected response)
O: Into<Request>, // Query Command configuration (opts)
{
// RPC Request

let request: Request = request.into();
let path = request.path.clone().unwrap(); // for the is_query_store_with_proof function

// Use the Tendermint-rs RPC client to do the query
let abci_response = chain
// Use the Tendermint-rs RPC client to do the query - Todo: generalize further for other type of chains
let response = chain
.rpc_client()
.abci_query(
request.path,
Expand All @@ -59,13 +56,11 @@ where
.await
.map_err(|e| error::Kind::Rpc.context(e))?;

if !abci_response.code.is_ok() {
if !response.code.is_ok() {
// Fail with response log
return Err(error::Kind::Rpc
.context(abci_response.log.to_string())
.into());
return Err(error::Kind::Rpc.context(response.log.to_string()).into());
}
if abci_response.value.is_empty() {
if response.value.is_empty() {
// Fail due to empty response value (nothing to decode).
return Err(error::Kind::EmptyResponseValue.into());
}
Expand All @@ -79,8 +74,6 @@ where

// Deserialize response data

// Poor man's serde(from='P') - this can be simplified if we use a serde-compatible protobuf implementation
let proto_type = R::decode(Bytes::from(abci_response.value))
.map_err(|e| error::Kind::ResponseParsing.context(e))?;
T::try_from(proto_type).map_err(|_e| error::Kind::ResponseParsing.into()) // Todo: Add context to error
let raw = T::raw_decode(response.value).map_err(|e| error::Kind::ResponseParsing.context(e))?;
T::validate(raw).map_err(|e| error::Kind::ResponseParsing.context(e).into())
}