fix error handling and counter value

anoma · Oct 28, 2023 · ec0391f · ec0391f
1 parent 5ee4ed4
commit ec0391f
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 89 deletions.
diff --git a/core/src/ledger/ibc/context/common.rs b/core/src/ledger/ibc/context/common.rs
@@ -161,25 +161,19 @@ pub trait IbcCommonContext: IbcStorageContext {
     /// Get the client update time
     fn client_update_time(&self, client_id: &ClientId) -> Result<Timestamp> {
         let key = storage::client_update_timestamp_key(client_id);
-        match self.read_bytes(&key)? {
-            Some(value) => {
-                let time = TmTime::decode_vec(&value).map_err(|_| {
-                    ContextError::from(ClientError::Other {
-                        description: format!(
-                            "Decoding the client update time failed: ID \
-                             {client_id}",
-                        ),
-                    })
-                })?;
-                Ok(time.into())
-            }
-            None => Err(ClientError::ClientSpecific {
+        let value =
+            self.read_bytes(&key)?.ok_or(ClientError::ClientSpecific {
                 description: format!(
                     "The client update time doesn't exist: ID {client_id}",
                 ),
-            }
-            .into()),
-        }
+            })?;
+        let time =
+            TmTime::decode_vec(&value).map_err(|_| ClientError::Other {
+                description: format!(
+                    "Decoding the client update time failed: ID {client_id}",
+                ),
+            })?;
+        Ok(time.into())
     }
 
     /// Store the client update time
@@ -189,41 +183,36 @@ pub trait IbcCommonContext: IbcStorageContext {
         timestamp: Timestamp,
     ) -> Result<()> {
         let key = storage::client_update_timestamp_key(client_id);
-        match timestamp.into_tm_time() {
-            Some(time) => self
-                .write_bytes(
-                    &key,
-                    time.encode_vec().expect("encoding shouldn't fail"),
-                )
-                .map_err(ContextError::from),
-            None => Err(ContextError::ClientError(ClientError::Other {
-                description: format!(
-                    "The client timestamp is invalid: ID {client_id}",
-                ),
-            })),
-        }
+        let time = timestamp.into_tm_time().ok_or(ClientError::Other {
+            description: format!(
+                "The client timestamp is invalid: ID {client_id}",
+            ),
+        })?;
+        self.write_bytes(
+            &key,
+            time.encode_vec().expect("encoding shouldn't fail"),
+        )
+        .map_err(ContextError::from)
     }
 
     /// Get the client update height
     fn client_update_height(&self, client_id: &ClientId) -> Result<Height> {
         let key = storage::client_update_height_key(client_id);
-        match self.read_bytes(&key)? {
-            Some(value) => Height::decode_vec(&value).map_err(|_| {
-                ClientError::Other {
-                    description: format!(
-                        "Decoding the client update height failed: ID \
-                         {client_id}",
-                    ),
-                }
-                .into()
-            }),
-            None => Err(ClientError::ClientSpecific {
+        let value = self.read_bytes(&key)?.ok_or({
+            ClientError::ClientSpecific {
                 description: format!(
                     "The client update height doesn't exist: ID {client_id}",
                 ),
             }
-            .into()),
-        }
+        })?;
+        Height::decode_vec(&value).map_err(|_| {
+            ClientError::Other {
+                description: format!(
+                    "Decoding the client update height failed: ID {client_id}",
+                ),
+            }
+            .into()
+        })
     }
 
     /// Get the timestamp on this chain
@@ -290,20 +279,19 @@ pub trait IbcCommonContext: IbcStorageContext {
     /// Get the ConnectionEnd
     fn connection_end(&self, conn_id: &ConnectionId) -> Result<ConnectionEnd> {
         let key = storage::connection_key(conn_id);
-        match self.read_bytes(&key)? {
-            Some(value) => ConnectionEnd::decode_vec(&value).map_err(|_| {
-                ConnectionError::Other {
-                    description: format!(
-                        "Decoding the connection end failed: ID {conn_id}",
-                    ),
-                }
-                .into()
-            }),
-            None => Err(ConnectionError::ConnectionNotFound {
+        let value = self.read_bytes(&key)?.ok_or(
+            ConnectionError::ConnectionNotFound {
                 connection_id: conn_id.clone(),
+            },
+        )?;
+        ConnectionEnd::decode_vec(&value).map_err(|_| {
+            ConnectionError::Other {
+                description: format!(
+                    "Decoding the connection end failed: ID {conn_id}",
+                ),
             }
-            .into()),
-        }
+            .into()
+        })
     }
 
     /// Store the ConnectionEnd
@@ -338,21 +326,20 @@ pub trait IbcCommonContext: IbcStorageContext {
         channel_id: &ChannelId,
     ) -> Result<ChannelEnd> {
         let key = storage::channel_key(port_id, channel_id);
-        match self.read_bytes(&key)? {
-            Some(value) => ChannelEnd::decode_vec(&value).map_err(|_| {
-                ChannelError::Other {
-                    description: format!(
-                        "Decoding the channel end failed: Key {key}",
-                    ),
-                }
-                .into()
-            }),
-            None => Err(ChannelError::ChannelNotFound {
-                port_id: port_id.clone(),
-                channel_id: channel_id.clone(),
+        let value =
+            self.read_bytes(&key)?
+                .ok_or(ChannelError::ChannelNotFound {
+                    port_id: port_id.clone(),
+                    channel_id: channel_id.clone(),
+                })?;
+        ChannelEnd::decode_vec(&value).map_err(|_| {
+            ChannelError::Other {
+                description: format!(
+                    "Decoding the channel end failed: Key {key}",
+                ),
             }
-            .into()),
-        }
+            .into()
+        })
     }
 
     /// Store the ChannelEnd
@@ -592,16 +579,8 @@ pub trait IbcCommonContext: IbcStorageContext {
 
     /// Read a counter
     fn read_counter(&self, key: &Key) -> Result<u64> {
-        match self.read_bytes(key)? {
-            Some(value) => {
-                let value: [u8; 8] =
-                    value.try_into().map_err(|_| ClientError::Other {
-                        description: format!(
-                            "The counter value wasn't u64: Key {key}",
-                        ),
-                    })?;
-                Ok(u64::from_be_bytes(value))
-            }
+        match self.read::<u64>(key)? {
+            Some(counter) => Ok(counter),
             None => unreachable!("the counter should be initialized"),
         }
     }
@@ -613,8 +592,7 @@ pub trait IbcCommonContext: IbcStorageContext {
             u64::checked_add(count, 1).ok_or_else(|| ClientError::Other {
                 description: format!("The counter overflow: Key {key}"),
             })?;
-        self.write_bytes(key, count.to_be_bytes())
-            .map_err(ContextError::from)
+        self.write(key, count).map_err(ContextError::from)
     }
 
     /// Write the IBC denom. The given address could be a non-Namada token.

diff --git a/shared/src/ledger/native_vp/ibc/mod.rs b/shared/src/ledger/native_vp/ibc/mod.rs
@@ -295,6 +295,7 @@ mod tests {
     use std::convert::TryFrom;
     use std::str::FromStr;
 
+    use borsh::BorshDeserialize;
     use borsh_ext::BorshSerializeExt;
     use namada_core::ledger::gas::TxGasMeter;
     use namada_core::ledger::governance::parameters::GovernanceParameters;
@@ -388,6 +389,7 @@ mod tests {
         get_epoch_duration_storage_key, get_max_expected_time_per_block_key,
     };
     use crate::ledger::parameters::EpochDuration;
+    use crate::ledger::storage_api::StorageRead;
     use crate::ledger::{ibc, pos};
     use crate::proof_of_stake::parameters::PosParams;
     use crate::proto::{Code, Data, Section, Signature, Tx};
@@ -571,8 +573,8 @@ mod tests {
         }
     }
 
-    fn increment_counter(wl_storage: &mut TestWlStorage, key: &Key) {
-        let count = match wl_storage.storage.read(key).expect("read failed").0 {
+    fn increment_sequence(wl_storage: &mut TestWlStorage, key: &Key) {
+        let count = match wl_storage.read_bytes(key).expect("read failed") {
             Some(value) => {
                 let count: [u8; 8] =
                     value.try_into().expect("decoding a count failed");
@@ -586,6 +588,19 @@ mod tests {
             .expect("write failed");
     }
 
+    fn increment_counter(wl_storage: &mut TestWlStorage, key: &Key) {
+        let count = match wl_storage.read_bytes(key).expect("read failed") {
+            Some(value) => {
+                u64::try_from_slice(&value).expect("invalid counter value")
+            }
+            None => unreachable!("The counter should be initialized"),
+        };
+        wl_storage
+            .write_log
+            .write(key, (count + 1).serialize_to_vec())
+            .expect("write failed");
+    }
+
     fn dummy_proof() -> CommitmentProofBytes {
         CommitmentProofBytes::try_from(vec![0]).unwrap()
     }
@@ -1558,13 +1573,13 @@ mod tests {
         let channel_id = get_channel_id();
         let port_id = msg.port_id_on_a.clone();
         let send_key = next_sequence_send_key(&port_id, &channel_id);
-        increment_counter(&mut wl_storage, &send_key);
+        increment_sequence(&mut wl_storage, &send_key);
         keys_changed.insert(send_key);
         let recv_key = next_sequence_recv_key(&port_id, &channel_id);
-        increment_counter(&mut wl_storage, &recv_key);
+        increment_sequence(&mut wl_storage, &recv_key);
         keys_changed.insert(recv_key);
         let ack_key = next_sequence_ack_key(&port_id, &channel_id);
-        increment_counter(&mut wl_storage, &ack_key);
+        increment_sequence(&mut wl_storage, &ack_key);
         keys_changed.insert(ack_key);
         // event
         let event = RawIbcEvent::OpenInitChannel(ChanOpenInit::new(
@@ -1681,13 +1696,13 @@ mod tests {
         let channel_id = get_channel_id();
         let port_id = msg.port_id_on_a.clone();
         let send_key = next_sequence_send_key(&port_id, &channel_id);
-        increment_counter(&mut wl_storage, &send_key);
+        increment_sequence(&mut wl_storage, &send_key);
         keys_changed.insert(send_key);
         let recv_key = next_sequence_recv_key(&port_id, &channel_id);
-        increment_counter(&mut wl_storage, &recv_key);
+        increment_sequence(&mut wl_storage, &recv_key);
         keys_changed.insert(recv_key);
         let ack_key = next_sequence_ack_key(&port_id, &channel_id);
-        increment_counter(&mut wl_storage, &ack_key);
+        increment_sequence(&mut wl_storage, &ack_key);
         keys_changed.insert(ack_key);
         // event
         let event = RawIbcEvent::OpenTryChannel(ChanOpenTry::new(

diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs
@@ -4,7 +4,7 @@ use std::collections::BTreeSet;
 use std::convert::TryInto;
 use std::num::TryFromIntError;
 
-use borsh::{BorshDeserialize, BorshSerialize};
+use borsh::BorshDeserialize;
 use borsh_ext::BorshSerializeExt;
 use masp_primitives::transaction::Transaction;
 use namada_core::ledger::gas::{GasMetering, TxGasMeter};