Implemented better Raw type handling (#156)

* Implemented better Raw type handling
* Some minor error-handling was fixed

modules/src/ics03_connection/connection.rs

@@ -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.
@@ -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());

modules/src/ics04_channel/channel.rs

@@ -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)]
@@ -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,

modules/src/lib.rs

@@ -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

modules/src/raw_decode.rs

@@ -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))
+    }
+}

relayer/cli/Cargo.toml

@@ -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"

relayer/cli/src/commands/query/channel.rs

@@ -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;
@@ -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),

relayer/cli/src/commands/query/connection.rs

@@ -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;
@@ -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),

relayer/relay/Cargo.toml

@@ -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]

relayer/relay/src/query.rs

@@ -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;
 
@@ -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,
@@ -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());
     }
@@ -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())
 }