Connection queries for Relayer (#136)

* Initial implementation of proto files required for connection. Proto files generated but replacing connection mod breaks the build

* Removed reference to MerklePrefix

* Implemented logic to proto_unmarshall the connection using prost #130

* Trimmed the proto file, fixed commitments. Ready for review. (#130)

* Fixed a couple of nits

* Fix for unused_qualifications h/t @gregszabo @andynog.

* Fixing unit testing since CommitPrefix was changed, failed to cargo build #136

* Replaced prost compilation with dependency on ibc-proto crate.

* Better error propagation in connection query.

* Fixed error handling when specifying height (-h) parameter in a query connection. Doesn't throw exception anymore. Show error message. #130

* Added validation for versions in ConnectionEnd unmarshalling

* Implemented fmt::Debug trait for CommitPrefix to properly display its value (string instead of vector of bytes). Assumes value is valid utf-8. #136

Co-authored-by: Andy Nogueira <me@andynogueira.dev>

modules/Cargo.toml

@@ -20,13 +20,18 @@ paths-ics = []
 tendermint = { git = "https://github.com/informalsystems/tendermint-rs.git", branch = "master" }
 tendermint-rpc = { git = "https://github.com/informalsystems/tendermint-rs.git", features=["client"] }
 
+# Proto definitions for all IBC-related interfaces, e.g., connections or channels.
+ibc-proto = "0.1.0"
+
 anomaly = "0.2.0"
 thiserror = "1.0.11"
 serde_derive = "1.0.104"
 serde = "1.0.104"
 serde_json = "1"
 tracing = "0.1.13"
+prost = "0.6.1"
+bytes = "0.5.5"
 
 [dev-dependencies]
 tokio = { version = "0.2", features = ["macros"] }
-subtle-encoding = { version = "0.5" }
+subtle-encoding = { version = "0.5" }

modules/src/error.rs

@@ -12,6 +12,10 @@ pub enum Kind {
     /// Event error (raised by the event monitor)
     #[error("Bad Notification")]
     Event,
+
+    /// Response parsing error
+    #[error("Could not parse/unmarshall response")]
+    ResponseParsing,
 }
 
 impl Kind {

modules/src/ics03_connection/connection.rs

@@ -4,6 +4,10 @@ use crate::ics23_commitment::CommitmentPrefix;
 use crate::ics24_host::identifier::{ClientId, ConnectionId};
 use serde_derive::{Deserialize, Serialize};
 
+// Import proto declarations.
+use ibc_proto::connection::ConnectionEnd as ProtoConnectionEnd;
+use ibc_proto::connection::Counterparty as ProtoCounterparty;
+
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct ConnectionEnd {
     state: State,
@@ -25,6 +29,37 @@ impl ConnectionEnd {
             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 == "" {
+            return Err(Kind::ConnectionNotFound.into());
+        }
+
+        // The Counterparty field is an Option, may be missing.
+        match pc.counterparty {
+            Some(cp) => {
+                let mut conn = ConnectionEnd::new(
+                    pc.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))?,
+                )
+                .unwrap();
+
+                // Set the state.
+                conn.set_state(State::from_i32(pc.state));
+                Ok(conn)
+            }
+
+            // If no counterparty was set, signal the error.
+            None => Err(Kind::MissingCounterparty.into()),
+        }
+    }
 }
 
 impl Connection for ConnectionEnd {
@@ -76,6 +111,17 @@ 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/error.rs

@@ -21,6 +21,15 @@ pub enum Kind {
 
     #[error("invalid proof")]
     InvalidProof,
+
+    #[error("queried for a non-existing connection")]
+    ConnectionNotFound,
+
+    #[error("missing counterparty")]
+    MissingCounterparty,
+
+    #[error("missing counterparty prefix")]
+    MissingCounterpartyPrefix,
 }
 
 impl Kind {

modules/src/ics03_connection/exported.rs

@@ -42,6 +42,15 @@ impl State {
             Self::Open => "OPEN",
         }
     }
+
+    pub fn from_i32(nr: i32) -> Self {
+        match nr {
+            1 => Self::Init,
+            2 => Self::TryOpen,
+            3 => Self::Open,
+            _ => Self::Uninitialized,
+        }
+    }
 }
 
 impl std::str::FromStr for State {

modules/src/ics03_connection/msgs.rs

@@ -308,7 +308,7 @@ mod tests {
             client_id: "srcclient".to_string(),
             counterparty_connection_id: "destconnection".to_string(),
             counterparty_client_id: "destclient".to_string(),
-            counterparty_commitment_prefix: CommitmentPrefix {},
+            counterparty_commitment_prefix: CommitmentPrefix::new(vec![]),
         };
 
         let tests: Vec<Test> = vec![
@@ -398,7 +398,7 @@ mod tests {
             client_id: "srcclient".to_string(),
             counterparty_connection_id: "destconnection".to_string(),
             counterparty_client_id: "destclient".to_string(),
-            counterparty_commitment_prefix: CommitmentPrefix {},
+            counterparty_commitment_prefix: CommitmentPrefix::new(vec![]),
             counterparty_versions: vec!["1.0.0".to_string()],
             proof_init: get_dummy_proof(),
             proof_consensus: get_dummy_proof(),

modules/src/ics03_connection/query.rs

@@ -11,6 +11,14 @@ use crate::path::{ConnectionPath, Path};
 use crate::query::{IbcQuery, IbcResponse};
 use crate::Height;
 
+use crate::ics03_connection::error::Error;
+
+// Import protobuf definitions.
+use ibc_proto::connection::ConnectionEnd as ProtoConnectionEnd;
+
+use bytes::Bytes;
+use prost::Message;
+
 pub struct QueryConnection {
     pub chain_height: Height,
     pub connection_id: ConnectionId,
@@ -79,14 +87,15 @@ impl IbcResponse<QueryConnection> for ConnectionResponse {
         query: QueryConnection,
         response: AbciQuery,
     ) -> Result<Self, error::Error> {
-        let connection = amino_unmarshal_binary_length_prefixed(&response.value)?;
-
-        Ok(ConnectionResponse::new(
-            query.connection_id,
-            connection,
-            response.proof,
-            response.height.into(),
-        ))
+        match proto_unmarshal(response.value) {
+            Ok(decoded_conn) => Ok(ConnectionResponse::new(
+                query.connection_id,
+                decoded_conn,
+                response.proof,
+                response.height.into(),
+            )),
+            Err(e) => Err(error::Kind::ResponseParsing.context(e).into()),
+        }
     }
 }
 
@@ -105,6 +114,8 @@ impl IdentifiedConnectionEnd {
     }
 }
 
-fn amino_unmarshal_binary_length_prefixed<T>(_bytes: &[u8]) -> Result<T, error::Error> {
-    todo!()
+fn proto_unmarshal(bytes: Vec<u8>) -> Result<ConnectionEnd, Error> {
+    let buf = Bytes::from(bytes);
+    let decoded = ProtoConnectionEnd::decode(buf).unwrap();
+    ConnectionEnd::from_proto_connection_end(decoded)
 }

modules/src/ics23_commitment/mod.rs

@@ -1,6 +1,7 @@
 use serde_derive::{Deserialize, Serialize};
 
 use crate::path::Path;
+use std::fmt;
 use tendermint::merkle::proof::Proof;
 
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
@@ -14,7 +15,7 @@ impl CommitmentPath {
     where
         P: Path,
     {
-        todo!()
+        CommitmentPath {}
     }
 }
 
@@ -31,7 +32,21 @@ impl CommitmentProof {
 }
 */
 
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
-pub struct CommitmentPrefix;
+#[derive(Clone, PartialEq, Serialize, Deserialize)]
+pub struct CommitmentPrefix(Vec<u8>);
 
-// TODO: impl CommitPrefix
+impl CommitmentPrefix {
+    pub fn new(content: Vec<u8>) -> Self {
+        Self { 0: content }
+    }
+}
+
+impl fmt::Debug for CommitmentPrefix {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let converted = String::from_utf8(self.clone().0);
+        match converted {
+            Ok(s) => write!(f, "{}", s),
+            Err(_e) => write!(f, "{:?}", &self.0),
+        }
+    }
+}

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

@@ -81,6 +81,7 @@ impl Runnable for QueryConnectionEndCmd {
         };
         status_info!("Options", "{:?}", opts);
 
+        let chain = TendermintChain::from_config(chain_config).unwrap();
         // run with proof:
         // cargo run --bin relayer -- -c simple_config.toml query connection end ibc0 ibconeconnection
         //
@@ -89,16 +90,16 @@ impl Runnable for QueryConnectionEndCmd {
         //
         // 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 chain = TendermintChain::from_config(chain_config).unwrap();
         let res = block_on(query_connection(
             &chain,
             opts.height,
             opts.connection_id.clone(),
             opts.proof,
         ));
+
         match res {
             Ok(cs) => status_info!("connection query result: ", "{:?}", cs.connection),
-            Err(e) => status_info!("connection query error: ", "{:?}", e),
+            Err(e) => status_info!("connection query error", "{}", e),
         }
     }
 }

relayer/relay/src/query.rs

@@ -26,7 +26,10 @@ where
         .map_err(|e| error::Kind::Rpc.context(e))?;
 
     if !abci_response.code.is_ok() {
-        todo!() // TODO: Fail with response log
+        // Fail with response log
+        return Err(error::Kind::Rpc
+            .context(abci_response.log.to_string())
+            .into());
     }
 
     // Data that is not from trusted node or subspace query needs verification