Avoided many unwraps() around Height conversion. (#301)

modules/src/context_mock.rs

@@ -150,7 +150,7 @@ impl ChainKeeper for MockChainContext {
 #[cfg(test)]
 mod tests {
     use crate::context_mock::MockChainContext;
-    use std::convert::TryInto;
+    use tendermint::block::Height;
 
     #[test]
     fn test_store_historical_info() {
@@ -169,22 +169,22 @@ mod tests {
         let tests: Vec<Test> = vec![
             Test {
                 name: "Add no prune".to_string(),
-                ctx: MockChainContext::new(3, 0_u64.try_into().unwrap()),
+                ctx: MockChainContext::new(3, Height::from(0_u32)),
                 args: [1].to_vec(),
             },
             Test {
                 name: "Add with prune".to_string(),
-                ctx: MockChainContext::new(3, 2_u64.try_into().unwrap()),
+                ctx: MockChainContext::new(3, Height::from(2_u32)),
                 args: [3, 4].to_vec(),
             },
             Test {
                 name: "Add with initial prune".to_string(),
-                ctx: MockChainContext::new(3, 10_u64.try_into().unwrap()),
+                ctx: MockChainContext::new(3, Height::from(10_u32)),
                 args: [11].to_vec(),
             },
             Test {
                 name: "Attempt to add non sequential headers".to_string(),
-                ctx: MockChainContext::new(3, 2_u64.try_into().unwrap()),
+                ctx: MockChainContext::new(3, Height::from(2_u32)),
                 args: [3, 5, 7].to_vec(),
             },
         ];

modules/src/events.rs

@@ -4,13 +4,13 @@ use crate::ics03_connection::events as ConnectionEvents;
 use crate::ics04_channel::events as ChannelEvents;
 use crate::ics20_fungible_token_transfer::events as TransferEvents;
 
-use tendermint::block;
 use tendermint_rpc::event::{Event as RpcEvent, EventData as RpcEventData};
 
 use anomaly::BoxError;
 use serde_derive::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::convert::{TryFrom, TryInto};
+use tendermint::block::Height;
 
 /// Events created by the IBC component of a chain, destined for a relayer.
 #[derive(Debug, Clone, Deserialize, Serialize)]
@@ -52,15 +52,15 @@ impl IBCEvent {
 
 #[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct RawObject {
-    pub height: block::Height,
+    pub height: Height,
     pub action: String,
     pub idx: usize,
     pub events: HashMap<String, Vec<String>>,
 }
 
 impl RawObject {
     pub fn new(
-        height: block::Height,
+        height: Height,
         action: String,
         idx: usize,
         events: HashMap<String, Vec<String>>,
@@ -131,13 +131,17 @@ pub fn get_all_events(result: RpcEvent) -> Result<Vec<IBCEvent>, String> {
 
         RpcEventData::Tx { .. } => {
             let events = &result.events.ok_or("missing events")?;
-            let height = events.get("tx.height").ok_or("tx.height")?[0]
+            let height_raw = events.get("tx.height").ok_or("tx.height")?[0]
                 .parse::<u64>()
                 .map_err(|e| e.to_string())?;
+            let height: Height = height_raw
+                .try_into()
+                .map_err(|_| "height parsing overflow")?;
+
             let actions_and_indices = extract_helper(&events)?;
             for action in actions_and_indices {
                 let ev = build_event(RawObject::new(
-                    height.try_into().unwrap(), // TODO: Handle overflow
+                    height,
                     action.0,
                     action.1 as usize,
                     events.clone(),

modules/src/ics02_client/client_def.rs

@@ -478,8 +478,9 @@ mod tests {
     use crate::ics07_tendermint::client_state::ClientState;
     use crate::ics07_tendermint::header::test_util::get_dummy_header;
     use prost_types::Any;
-    use std::convert::{TryFrom, TryInto};
+    use std::convert::TryFrom;
     use std::time::Duration;
+    use tendermint::block::Height;
 
     #[test]
     fn to_and_from_any() {
@@ -490,7 +491,7 @@ mod tests {
             unbonding_period: Duration::from_secs(128000),
             max_clock_drift: Duration::from_millis(3000),
             latest_height: tm_header.signed_header.header.height,
-            frozen_height: 0_u64.try_into().unwrap(),
+            frozen_height: Height::from(0_u32),
             allow_update_after_expiry: false,
             allow_update_after_misbehaviour: false,
         });

modules/src/ics02_client/handler/create_client.rs

@@ -53,7 +53,6 @@ pub fn process(
 
 #[cfg(test)]
 mod tests {
-    use std::convert::TryInto;
     use std::time::Duration;
 
     use super::*;
@@ -63,6 +62,7 @@ mod tests {
     use crate::ics07_tendermint::header::test_util::get_dummy_header;
     use crate::mock_client::header::MockHeader;
     use crate::mock_client::state::{MockClientState, MockConsensusState};
+    use tendermint::block::Height;
 
     #[test]
     fn test_create_client_ok() {
@@ -74,8 +74,8 @@ mod tests {
         let msg = MsgCreateAnyClient {
             client_id,
             client_type: ClientType::Mock,
-            client_state: MockClientState(MockHeader(42_u64.try_into().unwrap())).into(),
-            consensus_state: MockConsensusState(MockHeader(42_u64.try_into().unwrap())).into(),
+            client_state: MockClientState(MockHeader(Height::from(42_u32))).into(),
+            consensus_state: MockConsensusState(MockHeader(Height::from(42_u32))).into(),
             signer,
         };
 
@@ -108,7 +108,7 @@ mod tests {
 
     #[test]
     fn test_create_client_existing_client_type() {
-        let height = 42_u64.try_into().unwrap();
+        let height = Height::from(42_u32);
         let client_id: ClientId = "mockclient".parse().unwrap();
         let signer = get_dummy_account_id();
 
@@ -140,14 +140,14 @@ mod tests {
         let signer = get_dummy_account_id();
 
         let mut ctx = MockClientContext::default();
-        let height = 30_u64.try_into().unwrap();
+        let height = Height::from(30_u32);
         ctx.with_client_consensus_state(&client_id, height);
 
         let msg = MsgCreateAnyClient {
             client_id,
             client_type: ClientType::Tendermint,
-            client_state: MockClientState(MockHeader(42_u64.try_into().unwrap())).into(),
-            consensus_state: MockConsensusState(MockHeader(42_u64.try_into().unwrap())).into(),
+            client_state: MockClientState(MockHeader(Height::from(42_u32))).into(),
+            consensus_state: MockConsensusState(MockHeader(Height::from(42_u32))).into(),
             signer,
         };
 
@@ -164,30 +164,30 @@ mod tests {
     fn test_create_client_ok_multiple() {
         let existing_client_id: ClientId = "existingmockclient".parse().unwrap();
         let signer = get_dummy_account_id();
-        let height = 80_u64.try_into().unwrap();
+        let height = Height::from(80_u32);
         let mut ctx = MockClientContext::default();
         ctx.with_client_consensus_state(&existing_client_id, height);
 
         let create_client_msgs: Vec<MsgCreateAnyClient> = vec![
             MsgCreateAnyClient {
                 client_id: "newmockclient1".parse().unwrap(),
                 client_type: ClientType::Mock,
-                client_state: MockClientState(MockHeader(42_u64.try_into().unwrap())).into(),
-                consensus_state: MockConsensusState(MockHeader(42_u64.try_into().unwrap())).into(),
+                client_state: MockClientState(MockHeader(Height::from(42_u32))).into(),
+                consensus_state: MockConsensusState(MockHeader(Height::from(42_u32))).into(),
                 signer,
             },
             MsgCreateAnyClient {
                 client_id: "newmockclient2".parse().unwrap(),
                 client_type: ClientType::Mock,
-                client_state: MockClientState(MockHeader(42_u64.try_into().unwrap())).into(),
-                consensus_state: MockConsensusState(MockHeader(42_u64.try_into().unwrap())).into(),
+                client_state: MockClientState(MockHeader(Height::from(42_u32))).into(),
+                consensus_state: MockConsensusState(MockHeader(Height::from(42_u32))).into(),
                 signer,
             },
             MsgCreateAnyClient {
                 client_id: "newmockclient3".parse().unwrap(),
                 client_type: ClientType::Tendermint,
-                client_state: MockClientState(MockHeader(50_u64.try_into().unwrap())).into(),
-                consensus_state: MockConsensusState(MockHeader(50_u64.try_into().unwrap())).into(),
+                client_state: MockClientState(MockHeader(Height::from(50_u32))).into(),
+                consensus_state: MockConsensusState(MockHeader(Height::from(50_u32))).into(),
                 signer,
             },
         ]
@@ -237,7 +237,7 @@ mod tests {
             unbonding_period: Duration::from_secs(128000),
             max_clock_drift: Duration::from_millis(3000),
             latest_height: tm_header.signed_header.header.height,
-            frozen_height: 0_u64.try_into().unwrap(),
+            frozen_height: Height::from(0_u32),
             allow_update_after_expiry: false,
             allow_update_after_misbehaviour: false,
         });

modules/src/ics02_client/handler/update_client.rs

@@ -63,7 +63,6 @@ pub fn keep(keeper: &mut dyn ClientKeeper, result: UpdateClientResult) -> Result
 
 #[cfg(test)]
 mod tests {
-    use std::convert::TryInto;
     use tendermint::block::Height;
 
     use super::*;
@@ -78,11 +77,11 @@ mod tests {
         let signer = get_dummy_account_id();
 
         let mut ctx = MockClientContext::default();
-        ctx.with_client(&client_id, ClientType::Mock, 42_u64.try_into().unwrap());
+        ctx.with_client(&client_id, ClientType::Mock, Height::from(42_u32));
 
         let msg = MsgUpdateAnyClient {
             client_id,
-            header: MockHeader(46_u64.try_into().unwrap()).into(),
+            header: MockHeader(Height::from(46_u32)).into(),
             signer,
         };
 
@@ -112,11 +111,11 @@ mod tests {
         let signer = get_dummy_account_id();
 
         let mut ctx = MockClientContext::default();
-        ctx.with_client_consensus_state(&client_id, 42_u64.try_into().unwrap());
+        ctx.with_client_consensus_state(&client_id, Height::from(42_u32));
 
         let msg = MsgUpdateAnyClient {
             client_id: "nonexistingclient".parse().unwrap(),
-            header: MockHeader(46_u64.try_into().unwrap()).into(),
+            header: MockHeader(Height::from(46_u32)).into(),
             signer,
         };
 
@@ -141,8 +140,8 @@ mod tests {
         ];
         let signer = get_dummy_account_id();
 
-        let initial_height: Height = 45_u64.try_into().unwrap();
-        let update_height: Height = 49_u64.try_into().unwrap();
+        let initial_height = Height::from(45_u32);
+        let update_height = Height::from(49_u32);
 
         let mut ctx = MockClientContext::default();
 

modules/src/ics02_client/msgs.rs

@@ -138,7 +138,7 @@ pub struct MsgUpdateAnyClient {
 #[cfg(test)]
 mod tests {
     use ibc_proto::ibc::client::MsgCreateClient;
-    use std::convert::{TryFrom, TryInto};
+    use std::convert::TryFrom;
     use std::time::Duration;
 
     use crate::ics02_client::client_def::{AnyClientState, AnyConsensusState};
@@ -148,6 +148,7 @@ mod tests {
     use crate::ics07_tendermint::client_state::ClientState;
     use crate::ics07_tendermint::header::test_util::get_dummy_header;
     use crate::ics24_host::identifier::ClientId;
+    use tendermint::block::Height;
 
     #[test]
     fn to_and_from_any() {
@@ -161,7 +162,7 @@ mod tests {
             unbonding_period: Duration::from_secs(128000),
             max_clock_drift: Duration::from_millis(3000),
             latest_height: tm_header.signed_header.header.height,
-            frozen_height: 0_u64.try_into().unwrap(),
+            frozen_height: Height::from(0_u32),
             allow_update_after_expiry: false,
             allow_update_after_misbehaviour: false,
         });

modules/src/ics03_connection/msgs/conn_open_ack.rs

@@ -1,5 +1,5 @@
 use serde_derive::{Deserialize, Serialize};
-use std::convert::{TryFrom, TryInto};
+use std::convert::TryFrom;
 
 use ibc_proto::ibc::connection::MsgConnectionOpenAck as RawMsgConnectionOpenAck;
 use tendermint_proto::DomainType;
@@ -52,10 +52,10 @@ impl MsgConnectionOpenAck {
     }
 
     /// Getter for accessing the `consensus_height` field from this message. Returns the special
-    /// value `0` if this field is not set.
+    /// value `Height(0)` if this field is not set.
     pub fn consensus_height(&self) -> Height {
         match self.proofs.consensus_proof() {
-            None => 0_u64.try_into().unwrap(),
+            None => Height::from(0_u32),
             Some(p) => p.height(),
         }
     }

modules/src/ics03_connection/msgs/conn_open_try.rs

@@ -67,7 +67,7 @@ impl MsgConnectionOpenTry {
     /// value `0` if this field is not set.
     pub fn consensus_height(&self) -> Height {
         match self.proofs.consensus_proof() {
-            None => 0_u64.try_into().unwrap(),
+            None => Height::from(0_u32),
             Some(p) => p.height(),
         }
     }

modules/src/ics07_tendermint/client_state.rs

@@ -55,14 +55,14 @@ impl ClientState {
         }
 
         // Basic validation for the frozen_height parameter.
-        if frozen_height != 0_u64.try_into().unwrap() {
+        if frozen_height != Height::from(0_u32) {
             return Err(Kind::ValidationError
                 .context("ClientState cannot be frozen at creation time")
                 .into());
         }
 
-        // Basic validation for the frozen_height parameter.
-        if latest_height <= 0_u64.try_into().unwrap() {
+        // Validation for the latest_height parameter.
+        if latest_height <= Height::from(0_u32) {
             return Err(Kind::ValidationError
                 .context("ClientState latest height cannot be smaller than zero")
                 .into());
@@ -100,7 +100,7 @@ impl crate::ics02_client::state::ClientState for ClientState {
 
     fn is_frozen(&self) -> bool {
         // If 'frozen_height' is set to a non-zero value, then the client state is frozen.
-        self.frozen_height != 0_u64.try_into().unwrap()
+        self.frozen_height.value() != 0
     }
 }
 
@@ -168,7 +168,6 @@ fn decode_height(height: ibc_proto::ibc::client::Height) -> Height {
 
 #[cfg(test)]
 mod tests {
-    use std::convert::TryInto;
     use std::time::Duration;
 
     use crate::ics07_tendermint::client_state::ClientState;
@@ -212,8 +211,8 @@ mod tests {
             trusting_period: Duration::new(64000, 0),
             unbonding_period: Duration::new(128000, 0),
             max_clock_drift: Duration::new(3, 0),
-            latest_height: 10_u64.try_into().unwrap(),
-            frozen_height: 0_u64.try_into().unwrap(),
+            latest_height: Height::from(10_u32),
+            frozen_height: Height::from(0_u32),
             allow_update_after_expiry: false,
             allow_update_after_misbehaviour: false,
         };
@@ -233,7 +232,7 @@ mod tests {
             Test {
                 name: "Invalid frozen height parameter (should be 0)".to_string(),
                 params: ClientStateParams {
-                    frozen_height: 1_u64.try_into().unwrap(),
+                    frozen_height: Height::from(1_u32),
                     ..default_params.clone()
                 },
                 want_pass: false,

modules/src/ics07_tendermint/header.rs

@@ -43,12 +43,12 @@ impl crate::ics02_client::header::Header for Header {
 
 #[cfg(test)]
 pub mod test_util {
-    use std::convert::TryInto;
     use subtle_encoding::hex;
 
     use crate::ics07_tendermint::header::Header;
 
     use tendermint::block::signed_header::SignedHeader;
+    use tendermint::block::Height;
     use tendermint::validator::Info as ValidatorInfo;
     use tendermint::validator::Set as ValidatorSet;
     use tendermint::{vote, PublicKey};
@@ -82,7 +82,7 @@ pub mod test_util {
         Header {
             signed_header: shdr,
             validator_set: vs.clone(),
-            trusted_height: 9_u64.try_into().unwrap(),
+            trusted_height: Height::from(9_u32),
             trusted_validator_set: vs,
         }
     }