From 40486fe48d1cb620d66866d1bf35f62b10cfbca5 Mon Sep 17 00:00:00 2001
From: Adi Seredinschi <adi@informal.systems>
Date: Thu, 23 Jul 2020 17:30:38 +0200
Subject: [PATCH] Channel query validation & tests & identifier validation fix
 (#163 #148 #164)

* Refactored the location of JSON files for serialization testing (#118).

* Validation for channel end + tests template; needs more tests (#148).

* Refining tests for channel end.

* Aligned identifier validation with ICS024 & updated tests (#164).

* Added TODO re: version validation; fix VALID_SPECIAL_CHARS to remove space.

* Fixing the TODO for version field validation.
---
 relayer/cli/src/commands/query/channel.rs | 14 +++++++++++---
 relayer/relay/src/query.rs                | 16 +++++++---------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/relayer/cli/src/commands/query/channel.rs b/relayer/cli/src/commands/query/channel.rs
index 6bca582ac7..0e56d3eccc 100644
--- a/relayer/cli/src/commands/query/channel.rs
+++ b/relayer/cli/src/commands/query/channel.rs
@@ -116,8 +116,8 @@ impl Runnable for QueryChannelEndCmd {
         let res: Result<ChannelEnd, Error> = block_on(query(&chain, opts));
 
         match res {
-            Ok(cs) => status_info!("channel query result: ", "{:?}", cs),
-            Err(e) => status_info!("channel query error", "{}", e),
+            Ok(cs) => status_info!("Result for channel end query: ", "{:?}", cs),
+            Err(e) => status_info!("Error encountered on channel end query:", "{}", e),
         }
     }
 }
@@ -174,11 +174,19 @@ mod tests {
                 want_pass: false,
             },
             Test {
-                name: "Bad port, non-alpha".to_string(),
+                name: "Correct port (alphanumeric)".to_string(),
                 params: QueryChannelEndCmd {
                     port_id: Some("p34".to_string()),
                     ..default_params.clone()
                 },
+                want_pass: true,
+            },
+            Test {
+                name: "Incorrect port identifier (contains invalid character)".to_string(),
+                params: QueryChannelEndCmd {
+                    port_id: Some("p34^".to_string()),
+                    ..default_params.clone()
+                },
                 want_pass: false,
             },
             Test {
diff --git a/relayer/relay/src/query.rs b/relayer/relay/src/query.rs
index bf376adbf2..f0b4c0538a 100644
--- a/relayer/relay/src/query.rs
+++ b/relayer/relay/src/query.rs
@@ -23,7 +23,7 @@ pub struct Request {
 
 /// Whether or not this path requires proof verification.
 ///
-/// is_query_store_with_proofxpects a format like /<queryType>/<storeName>/<subpath>,
+/// is_query_store_with_proof expects a format like /<queryType>/<storeName>/<subpath>,
 /// where queryType must be "store" and subpath must be "key" to require a proof.
 fn is_query_store_with_proof(_path: &TendermintPath) -> bool {
     false
@@ -37,11 +37,11 @@ where
     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 - Todo: generalize further for other type of chains
+    // Use the Tendermint-rs RPC client to do the query.
+    // Todo: generalize further for other type of chains (#157).
     let response = chain
         .rpc_client()
         .abci_query(
@@ -57,7 +57,7 @@ where
         .map_err(|e| error::Kind::Rpc.context(e))?;
 
     if !response.code.is_ok() {
-        // Fail with response log
+        // Fail with response log.
         return Err(error::Kind::Rpc.context(response.log.to_string()).into());
     }
     if response.value.is_empty() {
@@ -65,14 +65,12 @@ where
         return Err(error::Kind::EmptyResponseValue.into());
     }
 
-    // Verify response proof
-
-    // Data that is not from trusted node or subspace query needs verification
+    // Verify response proof.
+    // Data that is not from trusted node or subspace query needs verification.
     if is_query_store_with_proof(&path) {
         todo!() // TODO: Verify proof
     }
 
-    // Deserialize response data
-
+    // Deserialize response data.
     T::deserialize(response.value)
 }