Merge pull request #29 from CosmWasm/key-decode-errors

More informative KeyDecodeErrors

crates/storey/src/containers/column.rs

@@ -7,7 +7,7 @@ use crate::encoding::{DecodableWith, EncodableWith};
 use crate::storage::{IterableStorage, StorageBranch};
 use crate::storage::{Storage, StorageMut};
 
-use super::{IterableAccessor, KeyDecodeError, Storable};
+use super::{IterableAccessor, Storable};
 
 const META_NEXT_IX: &[u8] = &[0];
 const META_LEN: &[u8] = &[1];
@@ -87,6 +87,7 @@ where
 {
     type AccessorT<S> = ColumnAccess<E, T, S>;
     type Key = u32;
+    type KeyDecodeError = ColumnKeyDecodeError;
     type Value = T;
     type ValueDecodeError = E::DecodeError;
 
@@ -97,7 +98,7 @@ where
         }
     }
 
-    fn decode_key(key: &[u8]) -> Result<Self::Key, KeyDecodeError> {
+    fn decode_key(key: &[u8]) -> Result<Self::Key, ColumnKeyDecodeError> {
         let key = decode_ix(key)?;
 
         Ok(key)
@@ -108,6 +109,10 @@ where
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)]
+#[error("invalid key length, expected 4 bytes of big-endian u32")]
+pub struct ColumnKeyDecodeError;
+
 /// An accessor for a `Column`.
 ///
 /// This type provides methods for interacting with the column in storage.
@@ -216,9 +221,9 @@ where
     }
 }
 
-fn decode_ix(key: &[u8]) -> Result<u32, KeyDecodeError> {
+fn decode_ix(key: &[u8]) -> Result<u32, ColumnKeyDecodeError> {
     if key.len() != 4 {
-        return Err(KeyDecodeError);
+        return Err(ColumnKeyDecodeError);
     }
 
     let row_key = u32::from_be_bytes([key[0], key[1], key[2], key[3]]);

crates/storey/src/containers/item.rs

@@ -4,7 +4,7 @@ use crate::encoding::{DecodableWith, EncodableWith, Encoding};
 use crate::storage::StorageBranch;
 use crate::storage::{Storage, StorageMut};
 
-use super::{KeyDecodeError, Storable};
+use super::Storable;
 
 /// A single item in the storage.
 ///
@@ -72,6 +72,7 @@ where
 {
     type AccessorT<S> = ItemAccess<E, T, S>;
     type Key = ();
+    type KeyDecodeError = ItemKeyDecodeError;
     type Value = T;
     type ValueDecodeError = E::DecodeError;
 
@@ -82,11 +83,11 @@ where
         }
     }
 
-    fn decode_key(key: &[u8]) -> Result<(), KeyDecodeError> {
+    fn decode_key(key: &[u8]) -> Result<(), ItemKeyDecodeError> {
         if key.is_empty() {
             Ok(())
         } else {
-            Err(KeyDecodeError)
+            Err(ItemKeyDecodeError)
         }
     }
 
@@ -95,6 +96,10 @@ where
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)]
+#[error("invalid key length, expected empty key")]
+pub struct ItemKeyDecodeError;
+
 /// An accessor for an `Item`.
 ///
 /// This type provides methods to get and set the value of the item.

crates/storey/src/containers/map.rs

@@ -4,7 +4,7 @@ use crate::storage::IterableStorage;
 use crate::storage::StorageBranch;
 
 use super::IterableAccessor;
-use super::{KeyDecodeError, Storable};
+use super::Storable;
 
 /// A map that stores values of type `V` under keys of type `K`.
 ///
@@ -51,6 +51,7 @@ impl<K, V> Map<K, V>
 where
     K: OwnedKey,
     V: Storable,
+    <V as Storable>::KeyDecodeError: std::fmt::Display,
 {
     /// Creates a new map with the given prefix.
     ///
@@ -92,9 +93,11 @@ impl<K, V> Storable for Map<K, V>
 where
     K: OwnedKey,
     V: Storable,
+    <V as Storable>::KeyDecodeError: std::fmt::Display,
 {
     type AccessorT<S> = MapAccess<K, V, S>;
     type Key = (K, V::Key);
+    type KeyDecodeError = MapKeyDecodeError<V::KeyDecodeError>;
     type Value = V::Value;
     type ValueDecodeError = V::ValueDecodeError;
 
@@ -105,15 +108,16 @@ where
         }
     }
 
-    fn decode_key(key: &[u8]) -> Result<Self::Key, KeyDecodeError> {
-        let len = *key.first().ok_or(KeyDecodeError)? as usize;
+    fn decode_key(key: &[u8]) -> Result<Self::Key, MapKeyDecodeError<V::KeyDecodeError>> {
+        let len = *key.first().ok_or(MapKeyDecodeError::EmptyKey)? as usize;
 
         if key.len() < len + 1 {
-            return Err(KeyDecodeError);
+            return Err(MapKeyDecodeError::KeyTooShort(len));
         }
 
-        let map_key = K::from_bytes(&key[1..len + 1]).map_err(|_| KeyDecodeError)?;
-        let rest = V::decode_key(&key[len + 1..])?;
+        let map_key =
+            K::from_bytes(&key[1..len + 1]).map_err(|_| MapKeyDecodeError::InvalidUtf8)?;
+        let rest = V::decode_key(&key[len + 1..]).map_err(MapKeyDecodeError::Inner)?;
 
         Ok((map_key, rest))
     }
@@ -123,6 +127,22 @@ where
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)]
+#[error("invalid key length, expected empty key")]
+pub enum MapKeyDecodeError<I: std::fmt::Display> {
+    #[error("empty key, expected length prefix (1 byte)")]
+    EmptyKey,
+
+    #[error("key too short, expected {0} bytes after length prefix")]
+    KeyTooShort(usize),
+
+    #[error("invalid UTF8")]
+    InvalidUtf8,
+
+    #[error("sub key decode error: {0}")]
+    Inner(I),
+}
+
 /// An accessor for a map.
 ///
 /// The accessor provides methods for interacting with the map in storage.
@@ -227,6 +247,7 @@ impl<K, V, S> IterableAccessor for MapAccess<K, V, S>
 where
     K: OwnedKey,
     V: Storable,
+    <V as Storable>::KeyDecodeError: std::fmt::Display,
     S: IterableStorage,
 {
     type StorableT = Map<K, V>;
@@ -242,7 +263,9 @@ pub trait Key {
 }
 
 pub trait OwnedKey: Key {
-    fn from_bytes(bytes: &[u8]) -> Result<Self, ()>
+    type Error;
+
+    fn from_bytes(bytes: &[u8]) -> Result<Self, Self::Error>
     where
         Self: Sized;
 }
@@ -253,12 +276,20 @@ impl Key for String {
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)]
+#[error("invalid UTF8")]
+pub struct InvalidUtf8;
+
 impl OwnedKey for String {
-    fn from_bytes(bytes: &[u8]) -> Result<Self, ()>
+    type Error = InvalidUtf8;
+
+    fn from_bytes(bytes: &[u8]) -> Result<Self, Self::Error>
     where
         Self: Sized,
     {
-        std::str::from_utf8(bytes).map(String::from).map_err(|_| ())
+        std::str::from_utf8(bytes)
+            .map(String::from)
+            .map_err(|_| InvalidUtf8)
     }
 }
 

crates/storey/src/containers/mod.rs

@@ -31,6 +31,9 @@ pub trait Storable {
     /// Containers that store one item and don't manage subkeys should use the `()` type here.
     type Key;
 
+    /// The error type for decoding keys.
+    type KeyDecodeError;
+
     /// The Value type for this collection/container. This is the type that will be used for
     /// value iteration.
     type Value;
@@ -47,7 +50,7 @@ pub trait Storable {
     ///
     /// This method is used in key iteration to provide a typed key rather than raw bytes
     /// to the user.
-    fn decode_key(key: &[u8]) -> Result<Self::Key, KeyDecodeError>;
+    fn decode_key(key: &[u8]) -> Result<Self::Key, Self::KeyDecodeError>;
 
     /// Decode a value from a byte slice.
     ///
@@ -56,14 +59,10 @@ pub trait Storable {
     fn decode_value(value: &[u8]) -> Result<Self::Value, Self::ValueDecodeError>;
 }
 
-/// A key decoding error.
-#[derive(Debug, PartialEq)]
-pub struct KeyDecodeError;
-
 /// A key-value pair decoding error.
 #[derive(Debug, PartialEq)]
-pub enum KVDecodeError<V> {
-    Key,
+pub enum KVDecodeError<K, V> {
+    Key(K),
     Value(V),
 }
 
@@ -133,12 +132,12 @@ where
     S: Storable,
     B: IterableStorage + 'i,
 {
-    type Item = Result<(S::Key, S::Value), KVDecodeError<S::ValueDecodeError>>;
+    type Item = Result<(S::Key, S::Value), KVDecodeError<S::KeyDecodeError, S::ValueDecodeError>>;
 
     fn next(&mut self) -> Option<Self::Item> {
         self.inner.next().map(|(k, v)| -> Self::Item {
             match (S::decode_key(&k), S::decode_value(&v)) {
-                (Err(_), _) => Err(KVDecodeError::Key),
+                (Err(e), _) => Err(KVDecodeError::Key(e)),
                 (_, Err(e)) => Err(KVDecodeError::Value(e)),
                 (Ok(k), Ok(v)) => Ok((k, v)),
             }
@@ -161,7 +160,7 @@ where
     S: Storable,
     B: IterableStorage + 'i,
 {
-    type Item = Result<S::Key, KeyDecodeError>;
+    type Item = Result<S::Key, S::KeyDecodeError>;
 
     fn next(&mut self) -> Option<Self::Item> {
         self.inner.next().map(|k| S::decode_key(&k))