From b8d85cf611189f6f73e141d6597d392995fcd46a Mon Sep 17 00:00:00 2001 From: Greg Szabo Date: Sat, 18 Jul 2020 00:52:42 -0400 Subject: [PATCH 1/3] Implemented better Raw type handling --- modules/src/ics03_connection/connection.rs | 7 +++-- modules/src/ics04_channel/channel.rs | 8 ++--- modules/src/lib.rs | 1 + modules/src/raw_decode.rs | 22 ++++++++++++++ relayer/cli/Cargo.toml | 2 -- relayer/cli/src/commands/query/channel.rs | 17 ++--------- relayer/cli/src/commands/query/connection.rs | 17 ++--------- relayer/relay/Cargo.toml | 2 -- relayer/relay/src/query.rs | 31 ++++++++------------ 9 files changed, 48 insertions(+), 59 deletions(-) create mode 100644 modules/src/raw_decode.rs diff --git a/modules/src/ics03_connection/connection.rs b/modules/src/ics03_connection/connection.rs index b9e7bc9f6f..311ad14d97 100644 --- a/modules/src/ics03_connection/connection.rs +++ b/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, } -impl TryFrom for ConnectionEnd { +impl RawDecode for ConnectionEnd { + type RawType = RawConnectionEnd; type Error = anomaly::Error; - - fn try_from(value: RawConnectionEnd) -> Result { + fn validate(value: RawConnectionEnd) -> Result { // Todo: Is validation complete here? (Code was moved from `from_proto_connection_end`.) if value.id == "" { return Err(Kind::ConnectionNotFound.into()); diff --git a/modules/src/ics04_channel/channel.rs b/modules/src/ics04_channel/channel.rs index a316b0a6b5..6f8d5400da 100644 --- a/modules/src/ics04_channel/channel.rs +++ b/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 for ChannelEnd { +impl RawDecode for ChannelEnd { + type RawType = RawChannel; type Error = anomaly::Error; - - fn try_from(value: RawChannel) -> Result { + fn validate(value: RawChannel) -> Result { // Todo: Do validation of data here. This is just an example implementation for testing. let ordering = match value.ordering { 0 => Order::None, diff --git a/modules/src/lib.rs b/modules/src/lib.rs index c0adcf14ef..02f4b760e4 100644 --- a/modules/src/lib.rs +++ b/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 diff --git a/modules/src/raw_decode.rs b/modules/src/raw_decode.rs new file mode 100644 index 0000000000..942d471ca5 --- /dev/null +++ b/modules/src/raw_decode.rs @@ -0,0 +1,22 @@ +//! 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(value: RawType) trait for validation. +//! Only serde does this for JSON and here we need to do it with protobuf. +use ::prost::Message; + +/// RawDecode trait needs to implement a validate() function and an Error type for the return of that function. +pub trait RawDecode: ::std::marker::Sized { + /// RawType defines the prost-compiled protobuf-defined Rust struct + type RawType: ::prost::Message + ::std::default::Default; + + /// Error defines the error type returned from validation. + type Error: ::std::convert::Into>; + + /// validate function will validate the incoming RawType and convert it to our internal type + fn validate(value: Self::RawType) -> Result; + + /// raw_decode function will decode the type from RawType using Prost + fn raw_decode(wire: ::std::vec::Vec) -> Self::RawType { + Self::RawType::decode(::bytes::Bytes::from(wire)).unwrap() // Todo: Error handling + } +} diff --git a/relayer/cli/Cargo.toml b/relayer/cli/Cargo.toml index e98a5840f1..ca76ba8830 100644 --- a/relayer/cli/Cargo.toml +++ b/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" diff --git a/relayer/cli/src/commands/query/channel.rs b/relayer/cli/src/commands/query/channel.rs index 45ffc25eca..0904a7bda5 100644 --- a/relayer/cli/src/commands/query/channel.rs +++ b/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 = block_on(query(&chain, opts)); match res { Ok(cs) => status_info!("connection query result: ", "{:?}", cs), diff --git a/relayer/cli/src/commands/query/connection.rs b/relayer/cli/src/commands/query/connection.rs index 98512207eb..4a2acedbaa 100644 --- a/relayer/cli/src/commands/query/connection.rs +++ b/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 = block_on(query(&chain, opts)); match res { Ok(cs) => status_info!("connection query result: ", "{:?}", cs), diff --git a/relayer/relay/Cargo.toml b/relayer/relay/Cargo.toml index 990af7d664..230f9879e9 100644 --- a/relayer/relay/Cargo.toml +++ b/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] diff --git a/relayer/relay/src/query.rs b/relayer/relay/src/query.rs index 6f688d7bd2..87cb49341a 100644 --- a/relayer/relay/src/query.rs +++ b/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(chain: &C, request: O) -> Result +pub async fn query(chain: &C, request: O) -> Result where - C: Chain, // Chain configuration - R: Message + Default, // Raw Struct type - T: TryFrom, // Internal Struct type - O: Into, // Query Command configuration (opts) + C: Chain, // Chain configuration + T: RawDecode, // Internal Struct type (expected response) + O: Into, // 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); + T::validate(raw).map_err(|e| error::Kind::ResponseParsing.context(e).into()) } From cff5500bd55fd982948a803e8c7a202b8b83e587 Mon Sep 17 00:00:00 2001 From: Greg Szabo Date: Sat, 18 Jul 2020 01:15:22 -0400 Subject: [PATCH 2/3] Error handling for prost::decode --- modules/src/raw_decode.rs | 18 ++++++++++++------ relayer/relay/src/query.rs | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/modules/src/raw_decode.rs b/modules/src/raw_decode.rs index 942d471ca5..b068851ae6 100644 --- a/modules/src/raw_decode.rs +++ b/modules/src/raw_decode.rs @@ -2,21 +2,27 @@ //! This is similar to the pattern of using the #[serde(from="RawType") derive for automatic //! conversion with the TryFrom::try_from(value: RawType) trait for validation. //! Only serde does this for JSON and here we need to do it with protobuf. -use ::prost::Message; +use prost::{Message, DecodeError}; +use std::marker::Sized; +use std::default::Default; +use std::convert::Into; +use std::error::Error; +use std::vec::Vec; +use bytes::Bytes; /// RawDecode trait needs to implement a validate() function and an Error type for the return of that function. -pub trait RawDecode: ::std::marker::Sized { +pub trait RawDecode: Sized { /// RawType defines the prost-compiled protobuf-defined Rust struct - type RawType: ::prost::Message + ::std::default::Default; + type RawType: Message + Default; /// Error defines the error type returned from validation. - type Error: ::std::convert::Into>; + type Error: Into>; /// validate function will validate the incoming RawType and convert it to our internal type fn validate(value: Self::RawType) -> Result; /// raw_decode function will decode the type from RawType using Prost - fn raw_decode(wire: ::std::vec::Vec) -> Self::RawType { - Self::RawType::decode(::bytes::Bytes::from(wire)).unwrap() // Todo: Error handling + fn raw_decode(wire: Vec) -> Result { + Self::RawType::decode(Bytes::from(wire)) } } diff --git a/relayer/relay/src/query.rs b/relayer/relay/src/query.rs index 87cb49341a..3aac49da9f 100644 --- a/relayer/relay/src/query.rs +++ b/relayer/relay/src/query.rs @@ -74,6 +74,6 @@ where // Deserialize response data - let raw = T::raw_decode(response.value); + 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()) } From 467c54b1004ab1d64521db6f1bd6c3966d94c50c Mon Sep 17 00:00:00 2001 From: Greg Szabo Date: Sat, 18 Jul 2020 01:22:14 -0400 Subject: [PATCH 3/3] fmt fix --- modules/src/raw_decode.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/src/raw_decode.rs b/modules/src/raw_decode.rs index b068851ae6..b28a34abb8 100644 --- a/modules/src/raw_decode.rs +++ b/modules/src/raw_decode.rs @@ -2,13 +2,13 @@ //! This is similar to the pattern of using the #[serde(from="RawType") derive for automatic //! conversion with the TryFrom::try_from(value: RawType) trait for validation. //! Only serde does this for JSON and here we need to do it with protobuf. -use prost::{Message, DecodeError}; -use std::marker::Sized; -use std::default::Default; +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; -use bytes::Bytes; /// RawDecode trait needs to implement a validate() function and an Error type for the return of that function. pub trait RawDecode: Sized {