Generic query, channel query and reduce code (#152)

* Added channel query and TryFromRaw trait

* Implemented generic query for abci queries, converted connection query and channel query to use that and disabled client query for now. Removed unused code.

* Fix for handling empty response value.

* Moved query parameters into the command parameters with Into. (#155)

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

modules/src/error.rs

@@ -16,6 +16,9 @@ pub enum Kind {
     /// Response parsing error
     #[error("Could not parse/unmarshall response")]
     ResponseParsing,
+
+    #[error("Empty response value")]
+    EmptyResponseValue,
 }
 
 impl Kind {

modules/src/ics02_client/query.rs

@@ -1,15 +1,10 @@
 use std::marker::PhantomData;
-use tendermint_rpc::endpoint::abci_query::AbciQuery;
-
-use tendermint::abci;
 
 use crate::ics23_commitment::{CommitmentPath, CommitmentProof};
 
-use crate::error;
-use crate::ics02_client::state::{ClientState, ConsensusState};
+//use crate::ics02_client::state::{ClientState, ConsensusState};
 use crate::ics24_host::identifier::ClientId;
-use crate::path::{ClientStatePath, ConsensusStatePath, Path};
-use crate::query::{IbcQuery, IbcResponse};
+use crate::path::{ClientStatePath, ConsensusStatePath};
 use crate::Height;
 
 pub struct QueryClientFullState<CLS> {
@@ -32,29 +27,6 @@ impl<CLS> QueryClientFullState<CLS> {
     }
 }
 
-impl<CLS> IbcQuery for QueryClientFullState<CLS>
-where
-    CLS: ClientState,
-{
-    type Response = ClientFullStateResponse<CLS>;
-
-    fn path(&self) -> abci::Path {
-        "/store/ibc/key".parse().unwrap()
-    }
-
-    fn height(&self) -> Height {
-        self.chain_height
-    }
-
-    fn prove(&self) -> bool {
-        self.prove
-    }
-
-    fn data(&self) -> Vec<u8> {
-        self.client_state_path.to_key().into()
-    }
-}
-
 pub struct ClientFullStateResponse<CLS> {
     pub client_state: CLS,
     pub proof: Option<CommitmentProof>,
@@ -80,27 +52,6 @@ impl<CLS> ClientFullStateResponse<CLS> {
     }
 }
 
-impl<CLS> IbcResponse<QueryClientFullState<CLS>> for ClientFullStateResponse<CLS>
-where
-    CLS: ClientState,
-{
-    fn from_abci_response(
-        query: QueryClientFullState<CLS>,
-        response: AbciQuery,
-    ) -> Result<Self, error::Error> {
-        Ok(ClientFullStateResponse::new(
-            query.client_id,
-            amino_unmarshal_binary_length_prefixed(&response.value)?,
-            response.proof,
-            response.height.into(),
-        ))
-    }
-}
-
-fn amino_unmarshal_binary_length_prefixed<T>(_bytes: &[u8]) -> Result<T, error::Error> {
-    todo!()
-}
-
 pub struct QueryClientConsensusState<CS> {
     pub chain_height: Height,
     pub client_id: ClientId,
@@ -128,29 +79,6 @@ impl<CS> QueryClientConsensusState<CS> {
     }
 }
 
-impl<CS> IbcQuery for QueryClientConsensusState<CS>
-where
-    CS: ConsensusState,
-{
-    type Response = ConsensusStateResponse<CS>;
-
-    fn path(&self) -> abci::Path {
-        "/store/ibc/key".parse().unwrap()
-    }
-
-    fn height(&self) -> Height {
-        self.chain_height
-    }
-
-    fn prove(&self) -> bool {
-        self.prove
-    }
-
-    fn data(&self) -> Vec<u8> {
-        self.consensus_state_path.to_key().into()
-    }
-}
-
 pub struct ConsensusStateResponse<CS> {
     pub consensus_state: CS,
     pub proof: Option<CommitmentProof>,
@@ -176,20 +104,3 @@ impl<CS> ConsensusStateResponse<CS> {
         }
     }
 }
-
-impl<CS> IbcResponse<QueryClientConsensusState<CS>> for ConsensusStateResponse<CS>
-where
-    CS: ConsensusState,
-{
-    fn from_abci_response(
-        query: QueryClientConsensusState<CS>,
-        response: AbciQuery,
-    ) -> Result<Self, error::Error> {
-        Ok(ConsensusStateResponse::new(
-            query.client_id,
-            amino_unmarshal_binary_length_prefixed(&response.value)?,
-            response.proof,
-            response.height.into(),
-        ))
-    }
-}

modules/src/ics03_connection/connection.rs

@@ -2,11 +2,13 @@ use super::exported::*;
 use crate::ics03_connection::error::{Error, Kind};
 use crate::ics23_commitment::CommitmentPrefix;
 use crate::ics24_host::identifier::{ClientId, ConnectionId};
+use crate::try_from_raw::TryFromRaw;
 use serde_derive::{Deserialize, Serialize};
 
 // Import proto declarations.
-use ibc_proto::connection::ConnectionEnd as ProtoConnectionEnd;
-use ibc_proto::connection::Counterparty as ProtoCounterparty;
+use ibc_proto::connection::ConnectionEnd as RawConnectionEnd;
+use ibc_proto::connection::Counterparty as RawCounterparty;
+use std::convert::TryFrom;
 
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct ConnectionEnd {
@@ -16,43 +18,31 @@ pub struct ConnectionEnd {
     versions: Vec<String>,
 }
 
-impl ConnectionEnd {
-    pub fn new(
-        client_id: ClientId,
-        counterparty: Counterparty,
-        versions: Vec<String>,
-    ) -> Result<Self, Error> {
-        Ok(Self {
-            state: State::Uninitialized,
-            client_id,
-            counterparty,
-            versions: validate_versions(versions).map_err(|e| Kind::InvalidVersion.context(e))?,
-        })
-    }
-
-    pub fn set_state(&mut self, new_state: State) {
-        self.state = new_state;
-    }
-
-    pub fn from_proto_connection_end(pc: ProtoConnectionEnd) -> Result<Self, Error> {
-        if pc.id == "" {
+impl TryFromRaw for ConnectionEnd {
+    type RawType = RawConnectionEnd;
+    type Error = anomaly::Error<Kind>;
+    fn try_from(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());
         }
 
         // The Counterparty field is an Option, may be missing.
-        match pc.counterparty {
+        match value.counterparty {
             Some(cp) => {
                 let mut conn = ConnectionEnd::new(
-                    pc.client_id
+                    value
+                        .client_id
                         .parse()
                         .map_err(|e| Kind::IdentifierError.context(e))?,
-                    Counterparty::from_proto_counterparty(cp)?,
-                    validate_versions(pc.versions).map_err(|e| Kind::InvalidVersion.context(e))?,
+                    Counterparty::try_from(cp)?,
+                    validate_versions(value.versions)
+                        .map_err(|e| Kind::InvalidVersion.context(e))?,
                 )
                 .unwrap();
 
                 // Set the state.
-                conn.set_state(State::from_i32(pc.state));
+                conn.set_state(State::from_i32(value.state));
                 Ok(conn)
             }
 
@@ -62,6 +52,25 @@ impl ConnectionEnd {
     }
 }
 
+impl ConnectionEnd {
+    pub fn new(
+        client_id: ClientId,
+        counterparty: Counterparty,
+        versions: Vec<String>,
+    ) -> Result<Self, Error> {
+        Ok(Self {
+            state: State::Uninitialized,
+            client_id,
+            counterparty,
+            versions: validate_versions(versions).map_err(|e| Kind::InvalidVersion.context(e))?,
+        })
+    }
+
+    pub fn set_state(&mut self, new_state: State) {
+        self.state = new_state;
+    }
+}
+
 impl Connection for ConnectionEnd {
     type ValidationError = Error;
 
@@ -95,6 +104,22 @@ pub struct Counterparty {
     prefix: CommitmentPrefix,
 }
 
+impl TryFrom<RawCounterparty> for Counterparty {
+    type Error = anomaly::Error<Kind>;
+
+    fn try_from(value: RawCounterparty) -> Result<Self, Self::Error> {
+        // Todo: Is validation complete here? (code was moved from `from_proto_counterparty`)
+        match value.prefix {
+            Some(prefix) => Counterparty::new(
+                value.client_id,
+                value.connection_id,
+                CommitmentPrefix::new(prefix.key_prefix),
+            ),
+            None => Err(Kind::MissingCounterpartyPrefix.into()),
+        }
+    }
+}
+
 impl Counterparty {
     pub fn new(
         client_id: String,
@@ -111,17 +136,6 @@ impl Counterparty {
             prefix,
         })
     }
-
-    pub fn from_proto_counterparty(pc: ProtoCounterparty) -> Result<Self, Error> {
-        match pc.prefix {
-            Some(prefix) => Counterparty::new(
-                pc.client_id,
-                pc.connection_id,
-                CommitmentPrefix::new(prefix.key_prefix),
-            ),
-            None => Err(Kind::MissingCounterpartyPrefix.into()),
-        }
-    }
 }
 
 impl ConnectionCounterparty for Counterparty {

modules/src/ics03_connection/mod.rs

@@ -5,4 +5,3 @@ pub mod error;
 pub mod events;
 pub mod exported;
 pub mod msgs;
-pub mod query;