Channel query validation & tests & identifier validation fix (#163 #1…

…48 #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.

modules/src/ics03_connection/connection.rs

@@ -36,8 +36,7 @@ impl TryFromRaw for ConnectionEnd {
                         .parse()
                         .map_err(|e| Kind::IdentifierError.context(e))?,
                     Counterparty::try_from(cp)?,
-                    validate_versions(value.versions)
-                        .map_err(|e| Kind::InvalidVersion.context(e))?,
+                    value.versions,
                 )
                 .unwrap();
 

modules/src/ics03_connection/exported.rs

@@ -33,7 +33,7 @@ pub enum State {
 }
 
 impl State {
-    /// Yields the state as a string
+    /// Yields the State as a string.
     pub fn as_string(&self) -> &'static str {
         match self {
             Self::Uninitialized => "UNINITIALIZED",
@@ -43,6 +43,7 @@ impl State {
         }
     }
 
+    /// Parses the State from a i32.
     pub fn from_i32(nr: i32) -> Self {
         match nr {
             1 => Self::Init,

modules/src/ics03_connection/msgs.rs

@@ -437,12 +437,13 @@ mod tests {
                 want_pass: false,
             },
             Test {
-                name: "Bad destination client id, name in uppercase".to_string(),
+                name: "Correct destination client id with lower/upper case and special chars"
+                    .to_string(),
                 params: ConOpenTryParams {
-                    counterparty_client_id: "BadClientId".to_string(),
+                    counterparty_client_id: "ClientId_".to_string(),
                     ..default_con_params.clone()
                 },
-                want_pass: false,
+                want_pass: true,
             },
             Test {
                 name: "Bad counterparty versions, empty versions vec".to_string(),

modules/src/ics04_channel/channel.rs

@@ -22,30 +22,48 @@ pub struct ChannelEnd {
 impl TryFromRaw for ChannelEnd {
     type RawType = RawChannel;
     type Error = anomaly::Error<Kind>;
+
     fn try_from(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,
-            1 => Order::Unordered,
-            2 => Order::Ordered,
-            _ => panic!("invalid order number"),
+        // Parse the ordering type. Propagate the error, if any, to our caller.
+        let chan_ordering = Order::from_i32(value.ordering)?;
+
+        let chan_state = State::from_i32(value.state)?;
+
+        // Pop out the Counterparty from the Option.
+        let counterparty = match value.counterparty {
+            Some(cp) => cp,
+            None => return Err(Kind::MissingCounterparty.into()),
         };
-        let counterparty = value.counterparty.unwrap();
+
+        // Assemble the 'remote' attribute of the Channel, which represents the Counterparty.
         let remote = Counterparty {
-            port_id: PortId::from_str(counterparty.port_id.as_str()).unwrap(),
-            channel_id: ChannelId::from_str(counterparty.channel_id.as_str()).unwrap(),
+            port_id: PortId::from_str(counterparty.port_id.as_str())
+                .map_err(|e| Kind::IdentifierError.context(e))?,
+            channel_id: ChannelId::from_str(counterparty.channel_id.as_str())
+                .map_err(|e| Kind::IdentifierError.context(e))?,
         };
+
+        // Parse each item in connection_hops into a ConnectionId.
         let connection_hops = value
             .connection_hops
             .into_iter()
-            .map(|e| ConnectionId::from_str(e.as_str()).unwrap())
-            .collect();
+            .map(|conn_id| ConnectionId::from_str(conn_id.as_str()))
+            .collect::<Result<Vec<_>, _>>()
+            .map_err(|e| Kind::IdentifierError.context(e))?;
+
+        // This field is supposed to be opaque to the core IBC protocol. Empty
+        // version is allowed by the specification (cf. ICS 004). No explicit validation necessary.
         let version = value.version;
-        Ok(ChannelEnd::new(ordering, remote, connection_hops, version))
+
+        let mut channel_end = ChannelEnd::new(chan_ordering, remote, connection_hops, version);
+        channel_end.set_state(chan_state);
+
+        Ok(channel_end)
     }
 }
 
 impl ChannelEnd {
+    /// Creates a new ChannelEnd in state Uninitialized and other fields parametrized.
     pub fn new(
         ordering: Order,
         remote: Counterparty,
@@ -60,6 +78,11 @@ impl ChannelEnd {
             version,
         }
     }
+
+    /// Updates the ChannelEnd to assume a new State 's'.
+    pub fn set_state(&mut self, s: State) {
+        self.state = s;
+    }
 }
 
 impl Channel for ChannelEnd {
@@ -136,3 +159,114 @@ impl ChannelCounterparty for Counterparty {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::ics04_channel::channel::ChannelEnd;
+    use crate::try_from_raw::TryFromRaw;
+    use ibc_proto::channel::Channel as RawChannel;
+    use ibc_proto::channel::Counterparty as RawCounterparty;
+
+    #[test]
+    fn channel_end_try_from_raw() {
+        let empty_raw_channel_end = RawChannel {
+            state: 0,
+            ordering: 0,
+            counterparty: None,
+            connection_hops: vec![],
+            version: "".to_string(),
+        };
+
+        let cparty = RawCounterparty {
+            port_id: "0123456789".into(),
+            channel_id: "0987654321".into(),
+        };
+
+        let raw_channel_end = RawChannel {
+            state: 0,
+            ordering: 0,
+            counterparty: Some(cparty),
+            connection_hops: vec![],
+            version: "".to_string(), // The version is not validated.
+        };
+
+        struct Test {
+            name: String,
+            params: RawChannel,
+            want_pass: bool,
+        }
+
+        let tests: Vec<Test> = vec![
+            Test {
+                name: "Raw channel end with missing counterparty".to_string(),
+                params: empty_raw_channel_end.clone(),
+                want_pass: false,
+            },
+            Test {
+                name: "Raw channel end with incorrect state".to_string(),
+                params: RawChannel {
+                    state: -1,
+                    ..raw_channel_end.clone()
+                },
+                want_pass: false,
+            },
+            Test {
+                name: "Raw channel end with incorrect ordering".to_string(),
+                params: RawChannel {
+                    ordering: -1,
+                    ..raw_channel_end.clone()
+                },
+                want_pass: false,
+            },
+            Test {
+                name: "Raw channel end with incorrect connection id in connection hops".to_string(),
+                params: RawChannel {
+                    connection_hops: vec!["connection*".to_string()].into_iter().collect(),
+                    ..raw_channel_end.clone()
+                },
+                want_pass: false,
+            },
+            Test {
+                name: "Raw channel end with incorrect connection id (has blank space)".to_string(),
+                params: RawChannel {
+                    connection_hops: vec!["con nection".to_string()].into_iter().collect(),
+                    ..raw_channel_end.clone()
+                },
+                want_pass: false,
+            },
+            Test {
+                name: "Raw channel end with two correct connection ids in connection hops"
+                    .to_string(),
+                params: RawChannel {
+                    connection_hops: vec!["connection1".to_string(), "connection2".to_string()]
+                        .into_iter()
+                        .collect(),
+                    ..raw_channel_end.clone()
+                },
+                want_pass: true,
+            },
+            Test {
+                name: "Raw channel end with correct params".to_string(),
+                params: raw_channel_end.clone(),
+                want_pass: true,
+            },
+        ]
+        .into_iter()
+        .collect();
+
+        for test in tests {
+            let p = test.params.clone();
+
+            let ce_result = ChannelEnd::try_from(p);
+
+            assert_eq!(
+                test.want_pass,
+                ce_result.is_ok(),
+                "ChannelEnd::try_from() failed for test {}, \nmsg{:?} with error {:?}",
+                test.name,
+                test.params.clone(),
+                ce_result.err(),
+            );
+        }
+    }
+}

modules/src/ics04_channel/error.rs

@@ -28,6 +28,9 @@ pub enum Kind {
 
     #[error("acknowledgement too long")]
     AcknowledgementTooLong,
+
+    #[error("missing counterparty")]
+    MissingCounterparty,
 }
 
 impl Kind {

modules/src/ics04_channel/exported.rs

@@ -40,6 +40,18 @@ impl State {
             Self::Closed => "CLOSED",
         }
     }
+
+    // Parses the State out from a i32.
+    pub fn from_i32(s: i32) -> Result<Self, error::Error> {
+        match s {
+            0 => Ok(Self::Uninitialized),
+            1 => Ok(Self::Init),
+            2 => Ok(Self::TryOpen),
+            3 => Ok(Self::Open),
+            4 => Ok(Self::Closed),
+            _ => fail!(error::Kind::UnknownState, s),
+        }
+    }
 }
 
 impl std::str::FromStr for State {
@@ -73,6 +85,16 @@ impl Order {
             Self::Ordered => "ORDERED",
         }
     }
+
+    // Parses the Order out from a i32.
+    pub fn from_i32(nr: i32) -> Result<Self, error::Error> {
+        match nr {
+            0 => Ok(Self::None),
+            1 => Ok(Self::Unordered),
+            2 => Ok(Self::Ordered),
+            _ => fail!(error::Kind::UnknownOrderType, nr),
+        }
+    }
 }
 
 impl std::str::FromStr for Order {