Merge pull request #59 from scrtlabs/dev-v0.5.0

Dev v0.5
scrtlabs · Sep 4, 2022 · 18af00d · 18af00d
2 parents d8b9879 + 19c2f0c
commit 18af00d
Show file tree

Hide file tree

Showing 8 changed files with 112 additions and 101 deletions.
diff --git a/Releases.md b/Releases.md
@@ -1,5 +1,17 @@
 # Release notes for the Secret Toolkit
 
+## v0.5.0
+This release includes some minor fixed to the storage package which required some breaking changes.
+We are releasing these breaking changes because we reached the conclusion that the current interfaces
+are prone to bugs, or inefficient. Unless you are using these specific interfaces, you should be able to upgrade from 0.4 without issues.
+
+### Breaking
+- Removed the implementations of Clone for storage types which are not useful and may cause data corruption if used incorrectly.
+- Changed `Keymap::insert` to take the item by reference rather than by value. This should reduce the cost of calling that function by avoiding cloning.
+
+### Features
+- Changed the implementation of the `add_prefix` methods in the storage package to use length prefixing, which should help avoid namespace collisions.
+
 ## secret-toolkit-storage v0.4.2
 
 * BUGFIX: implementation of `.clone` method fixed

diff --git a/packages/snip20/src/query.rs b/packages/snip20/src/query.rs
@@ -241,6 +241,7 @@ pub enum AuthenticatedQueryResponse {
         msg: String,
     },
 }
+
 /// wrapper to deserialize TokenInfo response
 #[derive(Deserialize)]
 pub struct TokenInfoResponse {

diff --git a/packages/storage/Cargo.toml b/packages/storage/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "secret-toolkit-storage"
-version = "0.4.2"
+version = "0.5.0"
 edition = "2018"
 authors = ["SCRT Labs <[email protected]>"]
 license-file = "../../LICENSE"

diff --git a/packages/storage/src/append_store.rs b/packages/storage/src/append_store.rs
@@ -11,6 +11,7 @@ use std::sync::Mutex;
 use serde::{de::DeserializeOwned, Serialize};
 
 use cosmwasm_std::{ReadonlyStorage, StdError, StdResult, Storage};
+use cosmwasm_storage::to_length_prefixed;
 
 use secret_toolkit_serialization::{Bincode2, Serde};
 
@@ -41,14 +42,13 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
             serialization_type: PhantomData,
         }
     }
+
     /// This is used to produce a new AppendListStorage. This can be used when you want to associate an AppendListStorage to each user
     /// and you still get to define the AppendListStorage as a static constant
     pub fn add_suffix(&self, suffix: &[u8]) -> Self {
-        let prefix = if let Some(prefix) = &self.prefix {
-            [prefix.clone(), suffix.to_vec()].concat()
-        } else {
-            [self.namespace.to_vec(), suffix.to_vec()].concat()
-        };
+        let suffix = to_length_prefixed(suffix);
+        let prefix = self.prefix.as_deref().unwrap_or(self.namespace);
+        let prefix = [prefix, suffix.as_slice()].concat();
         Self {
             namespace: self.namespace,
             prefix: Some(prefix),
@@ -82,10 +82,12 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
             }
         }
     }
+
     /// checks if the collection has any elements
     pub fn is_empty<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<bool> {
         Ok(self.get_len(storage)? == 0)
     }
+
     /// gets the element at pos if within bounds
     pub fn get_at<S: ReadonlyStorage>(&self, storage: &S, pos: u32) -> StdResult<T> {
         let len = self.get_len(storage)?;
@@ -94,6 +96,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
         }
         self.get_at_unchecked(storage, pos)
     }
+
     /// tries to get the element at pos
     fn get_at_unchecked<S: ReadonlyStorage>(&self, storage: &S, pos: u32) -> StdResult<T> {
         let key = pos.to_be_bytes();
@@ -108,10 +111,12 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
         let mut may_len = self.length.lock().unwrap();
         *may_len = Some(len);
     }
+
     /// Clear the collection
     pub fn clear<S: Storage>(&self, storage: &mut S) {
         self.set_len(storage, 0);
     }
+
     /// Replaces data at a position within bounds
     pub fn set_at<S: Storage>(&self, storage: &mut S, pos: u32, item: &T) -> StdResult<()> {
         let len = self.get_len(storage)?;
@@ -120,17 +125,20 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
         }
         self.set_at_unchecked(storage, pos, item)
     }
+
     /// Sets data at a given index
     fn set_at_unchecked<S: Storage>(&self, storage: &mut S, pos: u32, item: &T) -> StdResult<()> {
         self.save_impl(storage, &pos.to_be_bytes(), item)
     }
+
     /// Pushes an item to AppendStorage
     pub fn push<S: Storage>(&self, storage: &mut S, item: &T) -> StdResult<()> {
         let len = self.get_len(storage)?;
         self.set_at_unchecked(storage, len, item)?;
         self.set_len(storage, len + 1);
         Ok(())
     }
+
     /// Pops an item from AppendStore
     pub fn pop<S: Storage>(&self, storage: &mut S) -> StdResult<T> {
         if let Some(len) = self.get_len(storage)?.checked_sub(1) {
@@ -141,6 +149,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
             Err(StdError::generic_err("Can not pop from empty AppendStore"))
         }
     }
+
     /// Remove an element from the collection at the specified position.
     ///
     /// Removing the last element has a constant cost.
@@ -164,6 +173,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
         self.set_len(storage, len - 1);
         item
     }
+
     /// Returns a readonly iterator
     pub fn iter<S: ReadonlyStorage>(
         &self,
@@ -173,6 +183,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
         let iter = AppendStoreIter::new(self, storage, 0, len);
         Ok(iter)
     }
+
     /// does paging with the given parameters
     pub fn paging<S: ReadonlyStorage>(
         &self,
@@ -187,18 +198,6 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
     }
 }
 
-impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> Clone for AppendStore<'a, T, Ser> {
-    fn clone(&self) -> Self {
-        Self {
-            namespace: self.namespace,
-            prefix: self.prefix.clone(),
-            length: Mutex::new(None),
-            item_type: PhantomData,
-            serialization_type: PhantomData,
-        }
-    }
-}
-
 impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
     fn as_slice(&self) -> &[u8] {
         if let Some(prefix) = &self.prefix {
@@ -376,29 +375,29 @@ mod tests {
         let append_store: AppendStore<i32> = AppendStore::new(b"test");
 
         assert!(append_store.length.lock().unwrap().eq(&None));
-        assert_eq!(append_store.get_len(&mut storage)?, 0);
+        assert_eq!(append_store.get_len(&storage)?, 0);
         assert!(append_store.length.lock().unwrap().eq(&Some(0)));
 
         append_store.push(&mut storage, &1234)?;
         append_store.push(&mut storage, &2143)?;
         append_store.push(&mut storage, &3412)?;
         append_store.push(&mut storage, &4321)?;
         assert!(append_store.length.lock().unwrap().eq(&Some(4)));
-        assert_eq!(append_store.get_len(&mut storage)?, 4);
+        assert_eq!(append_store.get_len(&storage)?, 4);
 
         assert_eq!(append_store.pop(&mut storage), Ok(4321));
         assert_eq!(append_store.pop(&mut storage), Ok(3412));
         assert!(append_store.length.lock().unwrap().eq(&Some(2)));
-        assert_eq!(append_store.get_len(&mut storage)?, 2);
+        assert_eq!(append_store.get_len(&storage)?, 2);
 
         assert_eq!(append_store.pop(&mut storage), Ok(2143));
         assert_eq!(append_store.pop(&mut storage), Ok(1234));
         assert!(append_store.length.lock().unwrap().eq(&Some(0)));
-        assert_eq!(append_store.get_len(&mut storage)?, 0);
+        assert_eq!(append_store.get_len(&storage)?, 0);
 
         assert!(append_store.pop(&mut storage).is_err());
         assert!(append_store.length.lock().unwrap().eq(&Some(0)));
-        assert_eq!(append_store.get_len(&mut storage)?, 0);
+        assert_eq!(append_store.get_len(&storage)?, 0);
 
         Ok(())
     }

diff --git a/packages/storage/src/deque_store.rs b/packages/storage/src/deque_store.rs
@@ -12,6 +12,7 @@ use std::sync::Mutex;
 use serde::{de::DeserializeOwned, Serialize};
 
 use cosmwasm_std::{ReadonlyStorage, StdError, StdResult, Storage};
+use cosmwasm_storage::to_length_prefixed;
 
 use secret_toolkit_serialization::{Bincode2, Serde};
 
@@ -45,14 +46,13 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
             serialization_type: PhantomData,
         }
     }
+
     /// This is used to produce a new DequeStorage. This can be used when you want to associate an AppendListStorage to each user
     /// and you still get to define the DequeStorage as a static constant
     pub fn add_suffix(&self, suffix: &[u8]) -> Self {
-        let prefix = if let Some(prefix) = &self.prefix {
-            [prefix.clone(), suffix.to_vec()].concat()
-        } else {
-            [self.namespace.to_vec(), suffix.to_vec()].concat()
-        };
+        let suffix = to_length_prefixed(suffix);
+        let prefix = self.prefix.as_deref().unwrap_or(self.namespace);
+        let prefix = [prefix, suffix.as_slice()].concat();
         Self {
             namespace: self.namespace,
             prefix: Some(prefix),
@@ -79,6 +79,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
             },
         }
     }
+
     /// gets the offset from storage, and otherwise sets it to 0
     pub fn get_off<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
         let mut may_off = self.offset.lock().unwrap();
@@ -93,6 +94,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
             },
         }
     }
+
     /// gets offset or length
     fn _get_u32<S: ReadonlyStorage>(&self, storage: &S, key: &[u8]) -> StdResult<u32> {
         let num_key = [self.as_slice(), key].concat();
@@ -107,10 +109,12 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
             Ok(0)
         }
     }
+
     /// checks if the collection has any elements
     pub fn is_empty<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<bool> {
         Ok(self.get_len(storage)? == 0)
     }
+
     /// gets the element at pos if within bounds
     pub fn get_at<S: ReadonlyStorage>(&self, storage: &S, pos: u32) -> StdResult<T> {
         let len = self.get_len(storage)?;
@@ -119,37 +123,44 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
         }
         self.get_at_unchecked(storage, pos)
     }
+
     /// tries to get the element at pos
     fn get_at_unchecked<S: ReadonlyStorage>(&self, storage: &S, pos: u32) -> StdResult<T> {
         self.load_impl(storage, &self._get_offset_pos(storage, pos)?.to_be_bytes())
     }
+
     /// add the offset to the pos
     fn _get_offset_pos<S: ReadonlyStorage>(&self, storage: &S, pos: u32) -> StdResult<u32> {
         let off = self.get_off(storage)?;
         Ok(pos.overflowing_add(off).0)
     }
+
     /// Set the length of the collection
     fn set_len<S: Storage>(&self, storage: &mut S, len: u32) {
         let mut may_len = self.length.lock().unwrap();
         *may_len = Some(len);
         self._set_u32(storage, LEN_KEY, len)
     }
+
     /// Set the offset of the collection
     fn set_off<S: Storage>(&self, storage: &mut S, off: u32) {
         let mut may_off = self.offset.lock().unwrap();
         *may_off = Some(off);
         self._set_u32(storage, OFFSET_KEY, off)
     }
+
     /// Set the length or offset of the collection
     fn _set_u32<S: Storage>(&self, storage: &mut S, key: &[u8], num: u32) {
         let num_key = [self.as_slice(), key].concat();
         storage.set(&num_key, &num.to_be_bytes());
     }
+
     /// Clear the collection
     pub fn clear<S: Storage>(&self, storage: &mut S) {
         self.set_len(storage, 0);
         self.set_off(storage, 0);
     }
+
     /// Replaces data at a position within bounds
     pub fn set_at<S: Storage>(&self, storage: &mut S, pos: u32, item: &T) -> StdResult<()> {
         let len = self.get_len(storage)?;
@@ -158,6 +169,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
         }
         self.set_at_unchecked(storage, pos, item)
     }
+
     /// Sets data at a given index
     fn set_at_unchecked<S: Storage>(&self, storage: &mut S, pos: u32, item: &T) -> StdResult<()> {
         self.save_impl(
@@ -166,13 +178,15 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
             item,
         )
     }
+
     /// Pushes an item to the back
     pub fn push_back<S: Storage>(&self, storage: &mut S, item: &T) -> StdResult<()> {
         let len = self.get_len(storage)?;
         self.set_at_unchecked(storage, len, item)?;
         self.set_len(storage, len + 1);
         Ok(())
     }
+
     /// Pushes an item to the front
     pub fn push_front<S: Storage>(&self, storage: &mut S, item: &T) -> StdResult<()> {
         let off = self.get_off(storage)?;
@@ -182,6 +196,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
         self.set_len(storage, len + 1);
         Ok(())
     }
+
     /// Pops an item from the back
     pub fn pop_back<S: Storage>(&self, storage: &mut S) -> StdResult<T> {
         if let Some(len) = self.get_len(storage)?.checked_sub(1) {
@@ -192,6 +207,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
             Err(StdError::generic_err("Can not pop from empty DequeStore"))
         }
     }
+
     /// Pops an item from the front
     pub fn pop_front<S: Storage>(&self, storage: &mut S) -> StdResult<T> {
         if let Some(len) = self.get_len(storage)?.checked_sub(1) {
@@ -204,6 +220,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
             Err(StdError::generic_err("Can not pop from empty DequeStore"))
         }
     }
+
     /// Remove an element from the collection at the specified position.
     ///
     /// Removing an element from the head (first) or tail (last) has a constant cost.
@@ -238,12 +255,14 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
         self.set_len(storage, len - 1);
         item
     }
+
     /// Returns a readonly iterator
     pub fn iter<S: ReadonlyStorage>(&self, storage: &'a S) -> StdResult<DequeStoreIter<T, S, Ser>> {
         let len = self.get_len(storage)?;
         let iter = DequeStoreIter::new(self, storage, 0, len);
         Ok(iter)
     }
+
     /// does paging with the given parameters
     pub fn paging<S: ReadonlyStorage>(
         &self,
@@ -297,19 +316,6 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
     }
 }
 
-impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> Clone for DequeStore<'a, T, Ser> {
-    fn clone(&self) -> Self {
-        Self {
-            namespace: self.namespace,
-            prefix: self.prefix.clone(),
-            length: Mutex::new(None),
-            offset: Mutex::new(None),
-            item_type: PhantomData,
-            serialization_type: PhantomData,
-        }
-    }
-}
-
 /// An iterator over the contents of the deque store.
 pub struct DequeStoreIter<'a, T, S, Ser>
 where