From a7e7e191fc181252757c60a030ee6f61a51cd927 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 1 Oct 2020 22:54:42 +0200 Subject: [PATCH 01/33] Basic indexed_bucket compiles --- packages/storage/src/indexed_bucket.rs | 180 +++++++++++++++++++++++++ packages/storage/src/lib.rs | 2 + 2 files changed, 182 insertions(+) create mode 100644 packages/storage/src/indexed_bucket.rs diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs new file mode 100644 index 0000000000..57ee5e7ead --- /dev/null +++ b/packages/storage/src/indexed_bucket.rs @@ -0,0 +1,180 @@ +// this module requires iterator to be useful at all +#![cfg(feature = "iterator")] + +use cosmwasm_std::{to_vec, Order, StdError, StdResult, Storage, KV}; +use serde::de::DeserializeOwned; +use serde::Serialize; + +use crate::namespace_helpers::{ + get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, +}; +use crate::type_helpers::{deserialize_kv, may_deserialize, must_deserialize}; +use crate::{to_length_prefixed, to_length_prefixed_nested}; + +/// IndexedBucket works like a bucket but has a secondary index +/// This is a WIP. +/// Step 1 - allow exactly 1 secondary index, no multi-prefix on primary key +/// Step 2 - allow multiple named secondary indexes, no multi-prefix on primary key +/// Step 3 - allow multiple named secondary indexes, clean composite key support +/// +/// Current Status: 0 +pub struct IndexedBucket<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned, +{ + storage: &'a mut S, + prefix_pk: Vec, + prefix_idx: Vec, + indexer: fn(&T) -> Vec, +} + +impl<'a, S, T> IndexedBucket<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned, +{ + pub fn new(storage: &'a mut S, namespace: &[u8], indexer: fn(&T) -> Vec) -> Self { + IndexedBucket { + storage, + prefix_pk: to_length_prefixed_nested(&[namespace, b"pk"]), + prefix_idx: to_length_prefixed_nested(&[namespace, b"idx"]), + indexer, + } + } + + /// save will serialize the model and store, returns an error on serialization issues. + /// this must load the old value to update the indexes properly + /// if you loaded the old value earlier in the same function, use replace to avoid needless db reads + pub fn save(&mut self, key: &[u8], data: &T) -> StdResult<()> { + let old_data = self.may_load(key)?; + self.replace(key, Some(data), old_data.as_ref()) + } + + pub fn remove(&mut self, key: &[u8]) -> StdResult<()> { + let old_data = self.may_load(key)?; + self.replace(key, None, old_data.as_ref()) + } + + /// replace writes data to key. old_data must be the current stored value (from a previous load) + /// and is used to properly update the index. This is used by save, replace, and update + /// and can be called directly if you want to optimize + pub fn replace(&mut self, key: &[u8], data: Option<&T>, old_data: Option<&T>) -> StdResult<()> { + if let Some(old) = old_data { + let old_idx = (self.indexer)(old); + self.remove_from_index(&old_idx, key); + } + if let Some(updated) = data { + let new_idx = (self.indexer)(updated); + self.add_to_index(&new_idx, key); + set_with_prefix(self.storage, &self.prefix_pk, key, &to_vec(updated)?); + } else { + remove_with_prefix(self.storage, &self.prefix_pk, key); + } + Ok(()) + } + + // index is stored (namespace, idx): key -> b"1" + // idx is prefixed and appended to namespace + pub fn add_to_index(&mut self, idx: &[u8], key: &[u8]) { + // TODO: make this a bit cleaner + let mut index_space = self.prefix_idx.clone(); + let mut key_prefix = to_length_prefixed(idx); + index_space.append(&mut key_prefix); + set_with_prefix(self.storage, &self.index_space(idx), key, b"1"); + } + + // index is stored (namespace, idx): key -> b"1" + // idx is prefixed and appended to namespace + pub fn remove_from_index(&mut self, idx: &[u8], key: &[u8]) { + remove_with_prefix(self.storage, &self.index_space(idx), key); + } + + // TODO: make this a bit cleaner + fn index_space(&self, idx: &[u8]) -> Vec { + let mut index_space = self.prefix_idx.clone(); + let mut key_prefix = to_length_prefixed(idx); + index_space.append(&mut key_prefix); + index_space + } + + /// load will return an error if no data is set at the given key, or on parse error + pub fn load(&self, key: &[u8]) -> StdResult { + let value = get_with_prefix(self.storage, &self.prefix_pk, key); + must_deserialize(&value) + } + + /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. + /// returns an error on issues parsing + pub fn may_load(&self, key: &[u8]) -> StdResult> { + let value = get_with_prefix(self.storage, &self.prefix_pk, key); + may_deserialize(&value) + } + + /// iterates over the items in pk order + pub fn range<'b>( + &'b self, + start: Option<&[u8]>, + end: Option<&[u8]>, + order: Order, + ) -> Box>> + 'b> { + let mapped = range_with_prefix(self.storage, &self.prefix_pk, start, end, order) + .map(deserialize_kv::); + Box::new(mapped) + } + + /// returns all pks that where stored under this secondary index, always Ascending + /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply + pub fn pks_by_index<'b>(&'b self, idx: &[u8]) -> Box> + 'b> { + let start = self.index_space(idx); + // end is the next byte + let mut end = start.clone(); + let l = end.len(); + end[l - 1] += 1; + let mapped = range_with_prefix( + self.storage, + &self.prefix_idx, + Some(&start), + Some(&end), + Order::Ascending, + ) + .map(|(k, _)| k); + Box::new(mapped) + } + + /// returns all items that match this secondary index, always by pk Ascending + pub fn items_by_index<'b>( + &'b self, + idx: &[u8], + ) -> Box>> + 'b> { + let mapped = self.pks_by_index(idx).map(move |pk| { + let v = self.load(&pk)?; + Ok((pk, v)) + }); + Box::new(mapped) + } + + /// Loads the data, perform the specified action, and store the result + /// in the database. This is shorthand for some common sequences, which may be useful. + /// + /// If the data exists, `action(Some(value))` is called. Otherwise `action(None)` is called. + pub fn update(&mut self, key: &[u8], action: A) -> Result + where + A: FnOnce(Option) -> Result, + E: From, + { + // we cannot copy index and it is consumed by the action, so we cannot use input inside replace + // thus, we manually take care of removing the old index on success + let input = self.may_load(key)?; + let old_idx = input.as_ref().map(self.indexer); + + let output = action(input)?; + + // manually remove the old index if needed + if let Some(idx) = old_idx { + self.remove_from_index(&idx, key); + } + self.replace(key, Some(&output), None)?; + Ok(output) + } +} diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index e9973c5575..0494571086 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -1,4 +1,5 @@ mod bucket; +mod indexed_bucket; mod length_prefixed; mod namespace_helpers; mod prefixed_storage; @@ -9,6 +10,7 @@ mod type_helpers; mod typed; pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; +pub use indexed_bucket::IndexedBucket; pub use length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; pub use prefixed_storage::{prefixed, prefixed_read, PrefixedStorage, ReadonlyPrefixedStorage}; pub use sequence::{currval, nextval, sequence}; From 019038449dde3fba42b54a95e56807ba057554b4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 1 Oct 2020 23:04:27 +0200 Subject: [PATCH 02/33] Test query by index... fail --- packages/storage/src/indexed_bucket.rs | 58 +++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 57ee5e7ead..b4fd3d3e1d 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -17,7 +17,7 @@ use crate::{to_length_prefixed, to_length_prefixed_nested}; /// Step 2 - allow multiple named secondary indexes, no multi-prefix on primary key /// Step 3 - allow multiple named secondary indexes, clean composite key support /// -/// Current Status: 0 +/// Current Status: 1 pub struct IndexedBucket<'a, S, T> where S: Storage, @@ -178,3 +178,59 @@ where Ok(output) } } + +#[cfg(test)] +mod test { + use super::*; + + use cosmwasm_std::testing::MockStorage; + use serde::{Deserialize, Serialize}; + + #[derive(Serialize, Deserialize, PartialEq, Debug, Clone)] + struct Data { + pub name: String, + pub age: i32, + } + + fn by_name(data: &Data) -> Vec { + data.name.as_bytes().to_vec() + } + + #[test] + fn store_and_load_by_index() { + let mut store = MockStorage::new(); + let mut bucket = IndexedBucket::new(&mut store, b"data", by_name); + + // save data + let data = Data { + name: "Maria".to_string(), + age: 42, + }; + let pk: &[u8] = b"5627"; + bucket.save(pk, &data).unwrap(); + + // load it properly + let loaded = bucket.load(pk).unwrap(); + assert_eq!(data, loaded); + + // load it by secondary index (we must know how to compute this) + let marias: StdResult> = bucket.items_by_index(b"Maria").collect(); + let marias = marias.unwrap(); + assert_eq!(1, marias.len()); + let (k, v) = &marias[0]; + assert_eq!(pk, k.as_slice()); + assert_eq!(&data, v); + + // other index doesn't match (1 byte after) + let marias: StdResult> = bucket.items_by_index(b"Marib").collect(); + assert_eq!(0, marias.unwrap().len()); + + // other index doesn't match (1 byte before) + let marias: StdResult> = bucket.items_by_index(b"Mari`").collect(); + assert_eq!(0, marias.unwrap().len()); + + // other index doesn't match (longer) + let marias: StdResult> = bucket.items_by_index(b"Maria5").collect(); + assert_eq!(0, marias.unwrap().len()); + } +} From cce8040e7e4245fffa9f8796cecfc42771fe8e75 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 1 Oct 2020 23:14:45 +0200 Subject: [PATCH 03/33] Basic index query works --- packages/storage/src/indexed_bucket.rs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index b4fd3d3e1d..d00e11bd67 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -77,10 +77,6 @@ where // index is stored (namespace, idx): key -> b"1" // idx is prefixed and appended to namespace pub fn add_to_index(&mut self, idx: &[u8], key: &[u8]) { - // TODO: make this a bit cleaner - let mut index_space = self.prefix_idx.clone(); - let mut key_prefix = to_length_prefixed(idx); - index_space.append(&mut key_prefix); set_with_prefix(self.storage, &self.index_space(idx), key, b"1"); } @@ -127,18 +123,8 @@ where /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply pub fn pks_by_index<'b>(&'b self, idx: &[u8]) -> Box> + 'b> { let start = self.index_space(idx); - // end is the next byte - let mut end = start.clone(); - let l = end.len(); - end[l - 1] += 1; - let mapped = range_with_prefix( - self.storage, - &self.prefix_idx, - Some(&start), - Some(&end), - Order::Ascending, - ) - .map(|(k, _)| k); + let mapped = + range_with_prefix(self.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); Box::new(mapped) } From 29ab5e5e9806ade44979c5e457032f542ee97965 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 1 Oct 2020 23:20:50 +0200 Subject: [PATCH 04/33] Only export when iterator feature enabled --- packages/storage/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index 0494571086..ed5f08fe58 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -10,6 +10,7 @@ mod type_helpers; mod typed; pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; +#[cfg(feature = "iterator")] pub use indexed_bucket::IndexedBucket; pub use length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; pub use prefixed_storage::{prefixed, prefixed_read, PrefixedStorage, ReadonlyPrefixedStorage}; From 564a055969cd671f3297b87e84f0952f714ec544 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 00:57:38 +0200 Subject: [PATCH 05/33] WIP: Add multiple indexes - mutable/lifetime issue --- packages/storage/src/indexed_bucket.rs | 101 ++++++++++++++++--------- 1 file changed, 66 insertions(+), 35 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index d00e11bd67..3509b6001d 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -4,6 +4,7 @@ use cosmwasm_std::{to_vec, Order, StdError, StdResult, Storage, KV}; use serde::de::DeserializeOwned; use serde::Serialize; +use std::collections::BTreeMap; use crate::namespace_helpers::{ get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, @@ -17,16 +18,18 @@ use crate::{to_length_prefixed, to_length_prefixed_nested}; /// Step 2 - allow multiple named secondary indexes, no multi-prefix on primary key /// Step 3 - allow multiple named secondary indexes, clean composite key support /// -/// Current Status: 1 +/// Current Status: 1->2 pub struct IndexedBucket<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned, { storage: &'a mut S, + namespace: Vec, + // this can be computed from the namespace, but we cache it as it is used everywhere prefix_pk: Vec, - prefix_idx: Vec, - indexer: fn(&T) -> Vec, + // TODO: use the fully prefixed indexes as keys instead of names? (optimization) + indexers: BTreeMap Vec>, } impl<'a, S, T> IndexedBucket<'a, S, T> @@ -34,12 +37,32 @@ where S: Storage, T: Serialize + DeserializeOwned, { - pub fn new(storage: &'a mut S, namespace: &[u8], indexer: fn(&T) -> Vec) -> Self { + pub fn new(storage: &'a mut S, namespace: &[u8]) -> Self { IndexedBucket { storage, + namespace: namespace.to_vec(), prefix_pk: to_length_prefixed_nested(&[namespace, b"pk"]), - prefix_idx: to_length_prefixed_nested(&[namespace, b"idx"]), - indexer, + indexers: BTreeMap::new(), + } + } + + /// Usage: + /// let mut bucket = IndexedBucket::new(&mut storeage, b"foobar") + /// .with_index("name", |x| x.name.clone())? + /// .with_index("age", by_age)?; + pub fn with_index>( + mut self, + name: U, + indexer: fn(&T) -> Vec, + ) -> StdResult { + let name = name.into(); + let old = self.indexers.insert(name.clone(), indexer); + match old { + Some(_) => Err(StdError::generic_err(format!( + "Attempt to write index {} 2 times", + name + ))), + None => Ok(self), } } @@ -61,12 +84,17 @@ where /// and can be called directly if you want to optimize pub fn replace(&mut self, key: &[u8], data: Option<&T>, old_data: Option<&T>) -> StdResult<()> { if let Some(old) = old_data { - let old_idx = (self.indexer)(old); - self.remove_from_index(&old_idx, key); + // Note: this didn't work as we cannot mutably borrow self (remove_from_index) inside the iterator + for (name, indexer) in self.indexers.iter() { + let old_idx = indexer(old); + self.remove_from_index(&name, &old_idx, key); + } } if let Some(updated) = data { - let new_idx = (self.indexer)(updated); - self.add_to_index(&new_idx, key); + for (name, indexer) in self.indexers.iter() { + let new_idx = indexer(updated); + self.add_to_index(&name, &new_idx, key); + } set_with_prefix(self.storage, &self.prefix_pk, key, &to_vec(updated)?); } else { remove_with_prefix(self.storage, &self.prefix_pk, key); @@ -76,24 +104,27 @@ where // index is stored (namespace, idx): key -> b"1" // idx is prefixed and appended to namespace - pub fn add_to_index(&mut self, idx: &[u8], key: &[u8]) { - set_with_prefix(self.storage, &self.index_space(idx), key, b"1"); + pub fn add_to_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { + set_with_prefix(self.storage, &self.index_space(index_name, idx), key, b"1"); } // index is stored (namespace, idx): key -> b"1" // idx is prefixed and appended to namespace - pub fn remove_from_index(&mut self, idx: &[u8], key: &[u8]) { - remove_with_prefix(self.storage, &self.index_space(idx), key); + pub fn remove_from_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { + remove_with_prefix(self.storage, &self.index_space(index_name, idx), key); } - // TODO: make this a bit cleaner - fn index_space(&self, idx: &[u8]) -> Vec { - let mut index_space = self.prefix_idx.clone(); + fn index_space(&self, index_name: &str, idx: &[u8]) -> Vec { + let mut index_space = self.prefix_idx(index_name); let mut key_prefix = to_length_prefixed(idx); index_space.append(&mut key_prefix); index_space } + fn prefix_idx(&self, index_name: &str) -> Vec { + to_length_prefixed_nested(&[&self.namespace, index_name.as_bytes()]) + } + /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { let value = get_with_prefix(self.storage, &self.prefix_pk, key); @@ -121,8 +152,12 @@ where /// returns all pks that where stored under this secondary index, always Ascending /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply - pub fn pks_by_index<'b>(&'b self, idx: &[u8]) -> Box> + 'b> { - let start = self.index_space(idx); + pub fn pks_by_index<'b>( + &'b self, + index_name: &str, + idx: &[u8], + ) -> Box> + 'b> { + let start = self.index_space(index_name, idx); let mapped = range_with_prefix(self.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); Box::new(mapped) @@ -131,9 +166,10 @@ where /// returns all items that match this secondary index, always by pk Ascending pub fn items_by_index<'b>( &'b self, + index_name: &str, idx: &[u8], ) -> Box>> + 'b> { - let mapped = self.pks_by_index(idx).map(move |pk| { + let mapped = self.pks_by_index(index_name, idx).map(move |pk| { let v = self.load(&pk)?; Ok((pk, v)) }); @@ -149,18 +185,10 @@ where A: FnOnce(Option) -> Result, E: From, { - // we cannot copy index and it is consumed by the action, so we cannot use input inside replace - // thus, we manually take care of removing the old index on success let input = self.may_load(key)?; - let old_idx = input.as_ref().map(self.indexer); - let output = action(input)?; - - // manually remove the old index if needed - if let Some(idx) = old_idx { - self.remove_from_index(&idx, key); - } - self.replace(key, Some(&output), None)?; + // TODO: somehow optimize to avoid double-reading, but that requires may_load to return Cloneable + self.save(key, &output)?; Ok(output) } } @@ -185,7 +213,10 @@ mod test { #[test] fn store_and_load_by_index() { let mut store = MockStorage::new(); - let mut bucket = IndexedBucket::new(&mut store, b"data", by_name); + // TODO: add second index + let mut bucket = IndexedBucket::new(&mut store, b"data") + .with_index("name", by_name) + .unwrap(); // save data let data = Data { @@ -200,7 +231,7 @@ mod test { assert_eq!(data, loaded); // load it by secondary index (we must know how to compute this) - let marias: StdResult> = bucket.items_by_index(b"Maria").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Maria").collect(); let marias = marias.unwrap(); assert_eq!(1, marias.len()); let (k, v) = &marias[0]; @@ -208,15 +239,15 @@ mod test { assert_eq!(&data, v); // other index doesn't match (1 byte after) - let marias: StdResult> = bucket.items_by_index(b"Marib").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Marib").collect(); assert_eq!(0, marias.unwrap().len()); // other index doesn't match (1 byte before) - let marias: StdResult> = bucket.items_by_index(b"Mari`").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Mari`").collect(); assert_eq!(0, marias.unwrap().len()); // other index doesn't match (longer) - let marias: StdResult> = bucket.items_by_index(b"Maria5").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Maria5").collect(); assert_eq!(0, marias.unwrap().len()); } } From af066c0d78d649cb3d25b7c6a70253170d7c914e Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 01:08:18 +0200 Subject: [PATCH 06/33] Add support for multiple secondary indexes --- packages/storage/src/indexed_bucket.rs | 129 +++++++++++++++++-------- 1 file changed, 88 insertions(+), 41 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 3509b6001d..5617c0f6e1 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -11,6 +11,7 @@ use crate::namespace_helpers::{ }; use crate::type_helpers::{deserialize_kv, may_deserialize, must_deserialize}; use crate::{to_length_prefixed, to_length_prefixed_nested}; +use serde::export::PhantomData; /// IndexedBucket works like a bucket but has a secondary index /// This is a WIP. @@ -18,8 +19,20 @@ use crate::{to_length_prefixed, to_length_prefixed_nested}; /// Step 2 - allow multiple named secondary indexes, no multi-prefix on primary key /// Step 3 - allow multiple named secondary indexes, clean composite key support /// -/// Current Status: 1->2 +/// Current Status: 2 pub struct IndexedBucket<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned, +{ + core: Core<'a, S, T>, + // TODO: use the fully prefixed indexes as keys instead of names? (optimization) + indexers: BTreeMap Vec>, +} + +/// we pull out Core from IndexedBucket, so we can get a mutable reference to storage +/// while holding an immutable iterator over indexers +struct Core<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned, @@ -28,8 +41,62 @@ where namespace: Vec, // this can be computed from the namespace, but we cache it as it is used everywhere prefix_pk: Vec, - // TODO: use the fully prefixed indexes as keys instead of names? (optimization) - indexers: BTreeMap Vec>, + _phantom: PhantomData, +} + +impl<'a, S, T> Core<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned, +{ + pub fn set_pk(&mut self, key: &[u8], updated: &T) -> StdResult<()> { + Ok(set_with_prefix( + self.storage, + &self.prefix_pk, + key, + &to_vec(updated)?, + )) + } + + pub fn remove_pk(&mut self, key: &[u8]) { + remove_with_prefix(self.storage, &self.prefix_pk, key) + } + + /// load will return an error if no data is set at the given key, or on parse error + pub fn load(&self, key: &[u8]) -> StdResult { + let value = get_with_prefix(self.storage, &self.prefix_pk, key); + must_deserialize(&value) + } + + /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. + /// returns an error on issues parsing + pub fn may_load(&self, key: &[u8]) -> StdResult> { + let value = get_with_prefix(self.storage, &self.prefix_pk, key); + may_deserialize(&value) + } + + // index is stored (namespace, idx): key -> b"1" + // idx is prefixed and appended to namespace + pub fn add_to_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { + set_with_prefix(self.storage, &self.index_space(index_name, idx), key, b"1"); + } + + // index is stored (namespace, idx): key -> b"1" + // idx is prefixed and appended to namespace + pub fn remove_from_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { + remove_with_prefix(self.storage, &self.index_space(index_name, idx), key); + } + + fn index_space(&self, index_name: &str, idx: &[u8]) -> Vec { + let mut index_space = self.prefix_idx(index_name); + let mut key_prefix = to_length_prefixed(idx); + index_space.append(&mut key_prefix); + index_space + } + + fn prefix_idx(&self, index_name: &str) -> Vec { + to_length_prefixed_nested(&[&self.namespace, index_name.as_bytes()]) + } } impl<'a, S, T> IndexedBucket<'a, S, T> @@ -39,9 +106,12 @@ where { pub fn new(storage: &'a mut S, namespace: &[u8]) -> Self { IndexedBucket { - storage, - namespace: namespace.to_vec(), - prefix_pk: to_length_prefixed_nested(&[namespace, b"pk"]), + core: Core { + storage, + namespace: namespace.to_vec(), + prefix_pk: to_length_prefixed_nested(&[namespace, b"pk"]), + _phantom: Default::default(), + }, indexers: BTreeMap::new(), } } @@ -87,57 +157,34 @@ where // Note: this didn't work as we cannot mutably borrow self (remove_from_index) inside the iterator for (name, indexer) in self.indexers.iter() { let old_idx = indexer(old); - self.remove_from_index(&name, &old_idx, key); + self.core.remove_from_index(&name, &old_idx, key); } } if let Some(updated) = data { for (name, indexer) in self.indexers.iter() { let new_idx = indexer(updated); - self.add_to_index(&name, &new_idx, key); + self.core.add_to_index(&name, &new_idx, key); } - set_with_prefix(self.storage, &self.prefix_pk, key, &to_vec(updated)?); + self.core.set_pk(key, updated)?; } else { - remove_with_prefix(self.storage, &self.prefix_pk, key); + self.core.remove_pk(key); } Ok(()) } - // index is stored (namespace, idx): key -> b"1" - // idx is prefixed and appended to namespace - pub fn add_to_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { - set_with_prefix(self.storage, &self.index_space(index_name, idx), key, b"1"); - } - - // index is stored (namespace, idx): key -> b"1" - // idx is prefixed and appended to namespace - pub fn remove_from_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { - remove_with_prefix(self.storage, &self.index_space(index_name, idx), key); - } - - fn index_space(&self, index_name: &str, idx: &[u8]) -> Vec { - let mut index_space = self.prefix_idx(index_name); - let mut key_prefix = to_length_prefixed(idx); - index_space.append(&mut key_prefix); - index_space - } - - fn prefix_idx(&self, index_name: &str) -> Vec { - to_length_prefixed_nested(&[&self.namespace, index_name.as_bytes()]) - } - /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { - let value = get_with_prefix(self.storage, &self.prefix_pk, key); - must_deserialize(&value) + self.core.load(key) } /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing pub fn may_load(&self, key: &[u8]) -> StdResult> { - let value = get_with_prefix(self.storage, &self.prefix_pk, key); - may_deserialize(&value) + self.core.may_load(key) } + // TODO: move all iterators into core, just pass-through + /// iterates over the items in pk order pub fn range<'b>( &'b self, @@ -145,7 +192,7 @@ where end: Option<&[u8]>, order: Order, ) -> Box>> + 'b> { - let mapped = range_with_prefix(self.storage, &self.prefix_pk, start, end, order) + let mapped = range_with_prefix(self.core.storage, &self.core.prefix_pk, start, end, order) .map(deserialize_kv::); Box::new(mapped) } @@ -157,9 +204,9 @@ where index_name: &str, idx: &[u8], ) -> Box> + 'b> { - let start = self.index_space(index_name, idx); - let mapped = - range_with_prefix(self.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); + let start = self.core.index_space(index_name, idx); + let mapped = range_with_prefix(self.core.storage, &start, None, None, Order::Ascending) + .map(|(k, _)| k); Box::new(mapped) } From c199e68937c37aad5e3478ab22fec6de3d0826d4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 01:13:06 +0200 Subject: [PATCH 07/33] Pull more code into Core --- packages/storage/src/indexed_bucket.rs | 88 ++++++++++++++++++-------- 1 file changed, 60 insertions(+), 28 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 5617c0f6e1..9540e1c2d1 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -97,6 +97,44 @@ where fn prefix_idx(&self, index_name: &str) -> Vec { to_length_prefixed_nested(&[&self.namespace, index_name.as_bytes()]) } + + /// iterates over the items in pk order + pub fn range<'b>( + &'b self, + start: Option<&[u8]>, + end: Option<&[u8]>, + order: Order, + ) -> Box>> + 'b> { + let mapped = range_with_prefix(self.storage, &self.prefix_pk, start, end, order) + .map(deserialize_kv::); + Box::new(mapped) + } + + /// returns all pks that where stored under this secondary index, always Ascending + /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply + pub fn pks_by_index<'b>( + &'b self, + index_name: &str, + idx: &[u8], + ) -> Box> + 'b> { + let start = self.index_space(index_name, idx); + let mapped = + range_with_prefix(self.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); + Box::new(mapped) + } + + /// returns all items that match this secondary index, always by pk Ascending + pub fn items_by_index<'b>( + &'b self, + index_name: &str, + idx: &[u8], + ) -> Box>> + 'b> { + let mapped = self.pks_by_index(index_name, idx).map(move |pk| { + let v = self.load(&pk)?; + Ok((pk, v)) + }); + Box::new(mapped) + } } impl<'a, S, T> IndexedBucket<'a, S, T> @@ -172,6 +210,25 @@ where Ok(()) } + /// Loads the data, perform the specified action, and store the result + /// in the database. This is shorthand for some common sequences, which may be useful. + /// + /// If the data exists, `action(Some(value))` is called. Otherwise `action(None)` is called. + pub fn update(&mut self, key: &[u8], action: A) -> Result + where + A: FnOnce(Option) -> Result, + E: From, + { + let input = self.may_load(key)?; + let output = action(input)?; + // TODO: somehow optimize to avoid double-reading, but that requires may_load to return Cloneable + self.save(key, &output)?; + Ok(output) + } + + // Everything else, that doesn't touch indexers, is just pass-through from self.core, + // thus can be used from while iterating over indexes + /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { self.core.load(key) @@ -192,9 +249,7 @@ where end: Option<&[u8]>, order: Order, ) -> Box>> + 'b> { - let mapped = range_with_prefix(self.core.storage, &self.core.prefix_pk, start, end, order) - .map(deserialize_kv::); - Box::new(mapped) + self.core.range(start, end, order) } /// returns all pks that where stored under this secondary index, always Ascending @@ -204,10 +259,7 @@ where index_name: &str, idx: &[u8], ) -> Box> + 'b> { - let start = self.core.index_space(index_name, idx); - let mapped = range_with_prefix(self.core.storage, &start, None, None, Order::Ascending) - .map(|(k, _)| k); - Box::new(mapped) + self.core.pks_by_index(index_name, idx) } /// returns all items that match this secondary index, always by pk Ascending @@ -216,27 +268,7 @@ where index_name: &str, idx: &[u8], ) -> Box>> + 'b> { - let mapped = self.pks_by_index(index_name, idx).map(move |pk| { - let v = self.load(&pk)?; - Ok((pk, v)) - }); - Box::new(mapped) - } - - /// Loads the data, perform the specified action, and store the result - /// in the database. This is shorthand for some common sequences, which may be useful. - /// - /// If the data exists, `action(Some(value))` is called. Otherwise `action(None)` is called. - pub fn update(&mut self, key: &[u8], action: A) -> Result - where - A: FnOnce(Option) -> Result, - E: From, - { - let input = self.may_load(key)?; - let output = action(input)?; - // TODO: somehow optimize to avoid double-reading, but that requires may_load to return Cloneable - self.save(key, &output)?; - Ok(output) + self.core.items_by_index(index_name, idx) } } From b00e778d57cb9ae7ef76ba81fae57c76ea0513b9 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 01:17:58 +0200 Subject: [PATCH 08/33] Test two different indexes --- packages/storage/src/indexed_bucket.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 9540e1c2d1..3df363ab82 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -240,8 +240,6 @@ where self.core.may_load(key) } - // TODO: move all iterators into core, just pass-through - /// iterates over the items in pk order pub fn range<'b>( &'b self, @@ -254,6 +252,7 @@ where /// returns all pks that where stored under this secondary index, always Ascending /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply + /// TODO: return error if index_name is not registered? pub fn pks_by_index<'b>( &'b self, index_name: &str, @@ -289,12 +288,18 @@ mod test { data.name.as_bytes().to_vec() } + fn by_age(data: &Data) -> Vec { + data.age.to_be_bytes().into() + } + #[test] fn store_and_load_by_index() { let mut store = MockStorage::new(); // TODO: add second index let mut bucket = IndexedBucket::new(&mut store, b"data") .with_index("name", by_name) + .unwrap() + .with_index("age", by_age) .unwrap(); // save data @@ -328,5 +333,16 @@ mod test { // other index doesn't match (longer) let marias: StdResult> = bucket.items_by_index("name", b"Maria5").collect(); assert_eq!(0, marias.unwrap().len()); + + // match on proper age + let proper = 42u32.to_be_bytes(); + let marias: StdResult> = bucket.items_by_index("age", &proper).collect(); + let marias = marias.unwrap(); + assert_eq!(1, marias.len()); + + // no match on wrong age + let too_old = 43u32.to_be_bytes(); + let marias: StdResult> = bucket.items_by_index("age", &too_old).collect(); + assert_eq!(0, marias.unwrap().len()); } } From 54442d2f728290c1ce577883e1edda2af42af4a3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 01:19:13 +0200 Subject: [PATCH 09/33] Fix clippy warnings --- packages/storage/src/indexed_bucket.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 3df363ab82..b7b83fade9 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -50,12 +50,8 @@ where T: Serialize + DeserializeOwned, { pub fn set_pk(&mut self, key: &[u8], updated: &T) -> StdResult<()> { - Ok(set_with_prefix( - self.storage, - &self.prefix_pk, - key, - &to_vec(updated)?, - )) + set_with_prefix(self.storage, &self.prefix_pk, key, &to_vec(updated)?); + Ok(()) } pub fn remove_pk(&mut self, key: &[u8]) { From b950d019cff55446f4404be17b5b455acd0ca848 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 16:41:08 +0200 Subject: [PATCH 10/33] Only store namespace as &[u8] --- packages/storage/src/indexed_bucket.rs | 83 +++++++++++++++----------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index b7b83fade9..b9033c09c0 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -17,57 +17,59 @@ use serde::export::PhantomData; /// This is a WIP. /// Step 1 - allow exactly 1 secondary index, no multi-prefix on primary key /// Step 2 - allow multiple named secondary indexes, no multi-prefix on primary key -/// Step 3 - allow multiple named secondary indexes, clean composite key support +/// Step 3 - allow unique indexes - They store {pk: Vec, value: T} so we don't need to re-lookup +/// Step 4 - allow multiple named secondary indexes, clean composite key support /// /// Current Status: 2 -pub struct IndexedBucket<'a, S, T> +pub struct IndexedBucket<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned, { - core: Core<'a, S, T>, + core: Core<'a, 'b, S, T>, // TODO: use the fully prefixed indexes as keys instead of names? (optimization) indexers: BTreeMap Vec>, } +/// reserved name, no index may register +const PREFIX_PK: &[u8] = b"_pk"; + /// we pull out Core from IndexedBucket, so we can get a mutable reference to storage /// while holding an immutable iterator over indexers -struct Core<'a, S, T> +struct Core<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned, { storage: &'a mut S, - namespace: Vec, - // this can be computed from the namespace, but we cache it as it is used everywhere - prefix_pk: Vec, + namespace: &'b [u8], _phantom: PhantomData, } -impl<'a, S, T> Core<'a, S, T> +impl<'a, 'b, S, T> Core<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned, { pub fn set_pk(&mut self, key: &[u8], updated: &T) -> StdResult<()> { - set_with_prefix(self.storage, &self.prefix_pk, key, &to_vec(updated)?); + set_with_prefix(self.storage, &self.prefix_pk(), key, &to_vec(updated)?); Ok(()) } pub fn remove_pk(&mut self, key: &[u8]) { - remove_with_prefix(self.storage, &self.prefix_pk, key) + remove_with_prefix(self.storage, &self.prefix_pk(), key) } /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { - let value = get_with_prefix(self.storage, &self.prefix_pk, key); + let value = get_with_prefix(self.storage, &self.prefix_pk(), key); must_deserialize(&value) } /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing pub fn may_load(&self, key: &[u8]) -> StdResult> { - let value = get_with_prefix(self.storage, &self.prefix_pk, key); + let value = get_with_prefix(self.storage, &self.prefix_pk(), key); may_deserialize(&value) } @@ -91,28 +93,32 @@ where } fn prefix_idx(&self, index_name: &str) -> Vec { - to_length_prefixed_nested(&[&self.namespace, index_name.as_bytes()]) + to_length_prefixed_nested(&[self.namespace, index_name.as_bytes()]) + } + + fn prefix_pk(&self) -> Vec { + to_length_prefixed_nested(&[self.namespace, PREFIX_PK]) } /// iterates over the items in pk order - pub fn range<'b>( - &'b self, + pub fn range<'c>( + &'c self, start: Option<&[u8]>, end: Option<&[u8]>, order: Order, - ) -> Box>> + 'b> { - let mapped = range_with_prefix(self.storage, &self.prefix_pk, start, end, order) + ) -> Box>> + 'c> { + let mapped = range_with_prefix(self.storage, &self.prefix_pk(), start, end, order) .map(deserialize_kv::); Box::new(mapped) } /// returns all pks that where stored under this secondary index, always Ascending /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply - pub fn pks_by_index<'b>( - &'b self, + pub fn pks_by_index<'c>( + &'c self, index_name: &str, idx: &[u8], - ) -> Box> + 'b> { + ) -> Box> + 'c> { let start = self.index_space(index_name, idx); let mapped = range_with_prefix(self.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); @@ -120,11 +126,11 @@ where } /// returns all items that match this secondary index, always by pk Ascending - pub fn items_by_index<'b>( - &'b self, + pub fn items_by_index<'c>( + &'c self, index_name: &str, idx: &[u8], - ) -> Box>> + 'b> { + ) -> Box>> + 'c> { let mapped = self.pks_by_index(index_name, idx).map(move |pk| { let v = self.load(&pk)?; Ok((pk, v)) @@ -133,17 +139,16 @@ where } } -impl<'a, S, T> IndexedBucket<'a, S, T> +impl<'a, 'b, S, T> IndexedBucket<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned, { - pub fn new(storage: &'a mut S, namespace: &[u8]) -> Self { + pub fn new(storage: &'a mut S, namespace: &'b [u8]) -> Self { IndexedBucket { core: Core { storage, - namespace: namespace.to_vec(), - prefix_pk: to_length_prefixed_nested(&[namespace, b"pk"]), + namespace, _phantom: Default::default(), }, indexers: BTreeMap::new(), @@ -160,6 +165,12 @@ where indexer: fn(&T) -> Vec, ) -> StdResult { let name = name.into(); + if name.as_bytes() == PREFIX_PK { + return Err(StdError::generic_err( + "Index _pk is reserved for the primary key", + )); + } + let old = self.indexers.insert(name.clone(), indexer); match old { Some(_) => Err(StdError::generic_err(format!( @@ -237,32 +248,32 @@ where } /// iterates over the items in pk order - pub fn range<'b>( - &'b self, + pub fn range<'c>( + &'c self, start: Option<&[u8]>, end: Option<&[u8]>, order: Order, - ) -> Box>> + 'b> { + ) -> Box>> + 'c> { self.core.range(start, end, order) } /// returns all pks that where stored under this secondary index, always Ascending /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply /// TODO: return error if index_name is not registered? - pub fn pks_by_index<'b>( - &'b self, + pub fn pks_by_index<'c>( + &'c self, index_name: &str, idx: &[u8], - ) -> Box> + 'b> { + ) -> Box> + 'c> { self.core.pks_by_index(index_name, idx) } /// returns all items that match this secondary index, always by pk Ascending - pub fn items_by_index<'b>( - &'b self, + pub fn items_by_index<'c>( + &'c self, index_name: &str, idx: &[u8], - ) -> Box>> + 'b> { + ) -> Box>> + 'c> { self.core.items_by_index(index_name, idx) } } From 5aa9cbc4cad70bcd1d86ca74b59868435a17c308 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 17:32:04 +0200 Subject: [PATCH 11/33] MultiIndex using trait --- packages/storage/src/indexed_bucket.rs | 142 +++++++++++++++++++------ packages/storage/src/lib.rs | 2 +- 2 files changed, 112 insertions(+), 32 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index b9033c09c0..9c5f2072ea 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -4,7 +4,6 @@ use cosmwasm_std::{to_vec, Order, StdError, StdResult, Storage, KV}; use serde::de::DeserializeOwned; use serde::Serialize; -use std::collections::BTreeMap; use crate::namespace_helpers::{ get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, @@ -13,6 +12,11 @@ use crate::type_helpers::{deserialize_kv, may_deserialize, must_deserialize}; use crate::{to_length_prefixed, to_length_prefixed_nested}; use serde::export::PhantomData; +/// reserved name, no index may register +const PREFIX_PK: &[u8] = b"_pk"; +/// MARKER is stored in the multi-index as value, but we only look at the key (which is pk) +const MARKER: &[u8] = b"1"; + /// IndexedBucket works like a bucket but has a secondary index /// This is a WIP. /// Step 1 - allow exactly 1 secondary index, no multi-prefix on primary key @@ -21,25 +25,34 @@ use serde::export::PhantomData; /// Step 4 - allow multiple named secondary indexes, clean composite key support /// /// Current Status: 2 -pub struct IndexedBucket<'a, 'b, S, T> +pub struct IndexedBucket<'a, 'b, 'x, S, T> where - S: Storage, - T: Serialize + DeserializeOwned, + S: Storage + 'x, + T: Serialize + DeserializeOwned + Clone + 'x, { core: Core<'a, 'b, S, T>, - // TODO: use the fully prefixed indexes as keys instead of names? (optimization) - indexers: BTreeMap Vec>, + indexes: Vec + 'x>>, } -/// reserved name, no index may register -const PREFIX_PK: &[u8] = b"_pk"; +pub trait Index +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + // TODO: do we make this any Vec ? + fn name(&self) -> String; + fn index(&self, data: &T) -> Vec; + + fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()>; + fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()>; +} /// we pull out Core from IndexedBucket, so we can get a mutable reference to storage /// while holding an immutable iterator over indexers -struct Core<'a, 'b, S, T> +pub struct Core<'a, 'b, S, T> where S: Storage, - T: Serialize + DeserializeOwned, + T: Serialize + DeserializeOwned + Clone, { storage: &'a mut S, namespace: &'b [u8], @@ -49,7 +62,7 @@ where impl<'a, 'b, S, T> Core<'a, 'b, S, T> where S: Storage, - T: Serialize + DeserializeOwned, + T: Serialize + DeserializeOwned + Clone, { pub fn set_pk(&mut self, key: &[u8], updated: &T) -> StdResult<()> { set_with_prefix(self.storage, &self.prefix_pk(), key, &to_vec(updated)?); @@ -73,6 +86,16 @@ where may_deserialize(&value) } + // THis can be pulled into a trait. + // 1. index: fn(&T) -> Vec + // 2. store: needs access to core for storage and namespace + // + // 2 variants: + // * store (namespace, index_name, idx_value, key) -> b"1" - allows many and references pk + // * store (namespace, index_name, idx_value) -> {key, value} - allows one and copies pk and data + // // this would be the primary key - we abstract that too??? + // * store (namespace, index_name, pk) -> value - allows one with data + // index is stored (namespace, idx): key -> b"1" // idx is prefixed and appended to namespace pub fn add_to_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { @@ -139,10 +162,10 @@ where } } -impl<'a, 'b, S, T> IndexedBucket<'a, 'b, S, T> +impl<'a, 'b, 'x, S, T> IndexedBucket<'a, 'b, 'x, S, T> where S: Storage, - T: Serialize + DeserializeOwned, + T: Serialize + DeserializeOwned + Clone, { pub fn new(storage: &'a mut S, namespace: &'b [u8]) -> Self { IndexedBucket { @@ -151,13 +174,13 @@ where namespace, _phantom: Default::default(), }, - indexers: BTreeMap::new(), + indexes: vec![], } } /// Usage: /// let mut bucket = IndexedBucket::new(&mut storeage, b"foobar") - /// .with_index("name", |x| x.name.clone())? + /// .with_unique_index("name", |x| x.name.clone())? /// .with_index("age", by_age)?; pub fn with_index>( mut self, @@ -170,15 +193,18 @@ where "Index _pk is reserved for the primary key", )); } - - let old = self.indexers.insert(name.clone(), indexer); - match old { - Some(_) => Err(StdError::generic_err(format!( - "Attempt to write index {} 2 times", - name - ))), - None => Ok(self), + for existing in self.indexes.iter() { + if existing.name() == name { + return Err(StdError::generic_err(format!( + "Attempt to write index {} 2 times", + name + ))); + } } + + let index = MultiIndex::new(indexer, name); + self.indexes.push(Box::new(index)); + Ok(self) } /// save will serialize the model and store, returns an error on serialization issues. @@ -200,15 +226,13 @@ where pub fn replace(&mut self, key: &[u8], data: Option<&T>, old_data: Option<&T>) -> StdResult<()> { if let Some(old) = old_data { // Note: this didn't work as we cannot mutably borrow self (remove_from_index) inside the iterator - for (name, indexer) in self.indexers.iter() { - let old_idx = indexer(old); - self.core.remove_from_index(&name, &old_idx, key); + for index in self.indexes.iter() { + index.remove(&mut self.core, key, old)?; } } if let Some(updated) = data { - for (name, indexer) in self.indexers.iter() { - let new_idx = indexer(updated); - self.core.add_to_index(&name, &new_idx, key); + for index in self.indexes.iter() { + index.insert(&mut self.core, key, updated)?; } self.core.set_pk(key, updated)?; } else { @@ -228,7 +252,7 @@ where { let input = self.may_load(key)?; let output = action(input)?; - // TODO: somehow optimize to avoid double-reading, but that requires may_load to return Cloneable + // TODO: somehow optimize to avoid double-reading, but that requires may_load to return Clone self.save(key, &output)?; Ok(output) } @@ -278,6 +302,63 @@ where } } +//------- indexers ------// + +pub struct MultiIndex +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + idx_fn: fn(&T) -> Vec, + _name: String, + _phantom: PhantomData, +} + +impl MultiIndex +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + pub fn new>(idx_fn: fn(&T) -> Vec, name: U) -> Self { + MultiIndex { + idx_fn, + _name: name.into(), + _phantom: Default::default(), + } + } +} + +impl Index for MultiIndex +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + fn name(&self) -> String { + self._name.clone() + } + + fn index(&self, data: &T) -> Vec { + (self.idx_fn)(data) + } + + fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { + let idx = self.index(data); + set_with_prefix( + core.storage, + &core.index_space(&self._name, &idx), + pk, + MARKER, + ); + Ok(()) + } + + fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()> { + let idx = self.index(old_data); + remove_with_prefix(core.storage, &core.index_space(&self._name, &idx), pk); + Ok(()) + } +} + #[cfg(test)] mod test { use super::*; @@ -302,7 +383,6 @@ mod test { #[test] fn store_and_load_by_index() { let mut store = MockStorage::new(); - // TODO: add second index let mut bucket = IndexedBucket::new(&mut store, b"data") .with_index("name", by_name) .unwrap() diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index ed5f08fe58..27847f4d24 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -11,7 +11,7 @@ mod typed; pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; #[cfg(feature = "iterator")] -pub use indexed_bucket::IndexedBucket; +pub use indexed_bucket::{Core, IndexedBucket, MultiIndex}; pub use length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; pub use prefixed_storage::{prefixed, prefixed_read, PrefixedStorage, ReadonlyPrefixedStorage}; pub use sequence::{currval, nextval, sequence}; From 54d79deb06a1e8bb6db88ab9b942819ef7909974 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 17:48:49 +0200 Subject: [PATCH 12/33] Implement UniqueIndex --- packages/storage/src/indexed_bucket.rs | 73 +++++++++++++++++++++++++- packages/storage/src/lib.rs | 2 +- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 9c5f2072ea..f037664f80 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -1,9 +1,10 @@ // this module requires iterator to be useful at all #![cfg(feature = "iterator")] -use cosmwasm_std::{to_vec, Order, StdError, StdResult, Storage, KV}; +use cosmwasm_std::{to_vec, Binary, Order, StdError, StdResult, Storage, KV}; use serde::de::DeserializeOwned; use serde::Serialize; +// use serde::{Deserialize, Serialize}; use crate::namespace_helpers::{ get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, @@ -359,6 +360,76 @@ where } } +pub struct UniqueIndex +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + idx_fn: fn(&T) -> Vec, + _name: String, + _phantom: PhantomData, +} + +#[derive(Serialize, Clone)] +pub struct UniqueRef +where + T: Serialize + DeserializeOwned + Clone, +{ + pk: Binary, + value: T, +} + +impl UniqueIndex +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + pub fn new>(idx_fn: fn(&T) -> Vec, name: U) -> Self { + UniqueIndex { + idx_fn, + _name: name.into(), + _phantom: Default::default(), + } + } +} + +impl Index for UniqueIndex +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + fn name(&self) -> String { + self._name.clone() + } + + fn index(&self, data: &T) -> Vec { + (self.idx_fn)(data) + } + + // we store (namespace, index_name, idx_value) -> { pk, value } + fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { + let idx = self.index(data); + let reference = UniqueRef:: { + pk: pk.into(), + value: data.clone(), + }; + set_with_prefix( + core.storage, + &core.prefix_idx(&self._name), + &idx, + &to_vec(&reference)?, + ); + Ok(()) + } + + // we store (namespace, index_name, idx_value) -> { pk, value } + fn remove(&self, core: &mut Core, _pk: &[u8], old_data: &T) -> StdResult<()> { + let idx = self.index(old_data); + remove_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); + Ok(()) + } +} + #[cfg(test)] mod test { use super::*; diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index 27847f4d24..0394cdf73c 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -11,7 +11,7 @@ mod typed; pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; #[cfg(feature = "iterator")] -pub use indexed_bucket::{Core, IndexedBucket, MultiIndex}; +pub use indexed_bucket::{Core, IndexedBucket, MultiIndex, UniqueIndex}; pub use length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; pub use prefixed_storage::{prefixed, prefixed_read, PrefixedStorage, ReadonlyPrefixedStorage}; pub use sequence::{currval, nextval, sequence}; From c265ff51b574166c4ef5f5ac2530be983541a59e Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 2 Oct 2020 18:10:37 +0200 Subject: [PATCH 13/33] Implement index-specific getters --- packages/storage/src/indexed_bucket.rs | 92 +++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 10 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index f037664f80..674286065a 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -1,10 +1,10 @@ // this module requires iterator to be useful at all #![cfg(feature = "iterator")] -use cosmwasm_std::{to_vec, Binary, Order, StdError, StdResult, Storage, KV}; +use cosmwasm_std::{from_slice, to_vec, Binary, Order, StdError, StdResult, Storage, KV}; use serde::de::DeserializeOwned; -use serde::Serialize; -// use serde::{Deserialize, Serialize}; +// use serde::Serialize; +use serde::{Deserialize, Serialize}; use crate::namespace_helpers::{ get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, @@ -46,6 +46,20 @@ where fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()>; fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()>; + + // these should be implemented by all + fn pks_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box> + 'c>; + + /// returns all items that match this secondary index, always by pk Ascending + fn items_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box>> + 'c>; } /// we pull out Core from IndexedBucket, so we can get a mutable reference to storage @@ -252,9 +266,9 @@ where E: From, { let input = self.may_load(key)?; + let old_val = input.clone(); let output = action(input)?; - // TODO: somehow optimize to avoid double-reading, but that requires may_load to return Clone - self.save(key, &output)?; + self.replace(key, Some(&output), old_val.as_ref())?; Ok(output) } @@ -358,6 +372,30 @@ where remove_with_prefix(core.storage, &core.index_space(&self._name, &idx), pk); Ok(()) } + + fn pks_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box> + 'c> { + let start = core.index_space(&self._name, idx); + let mapped = + range_with_prefix(core.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); + Box::new(mapped) + } + + /// returns all items that match this secondary index, always by pk Ascending + fn items_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box>> + 'c> { + let mapped = self.pks_by_index(core, idx).map(move |pk| { + let v = core.load(&pk)?; + Ok((pk, v)) + }); + Box::new(mapped) + } } pub struct UniqueIndex @@ -370,11 +408,8 @@ where _phantom: PhantomData, } -#[derive(Serialize, Clone)] -pub struct UniqueRef -where - T: Serialize + DeserializeOwned + Clone, -{ +#[derive(Deserialize, Serialize, Clone)] +pub struct UniqueRef { pk: Binary, value: T, } @@ -428,6 +463,43 @@ where remove_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); Ok(()) } + + // there is exactly 0 or 1 here... + fn pks_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box> + 'c> { + let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); + let res = match data { + Some(bin) => vec![bin], + None => vec![], + }; + let mapped = res.into_iter().map(|bin| { + // TODO: update types so we can return error here + let parsed: UniqueRef = from_slice(&bin).unwrap(); + parsed.pk.into_vec() + }); + Box::new(mapped) + } + + /// returns all items that match this secondary index, always by pk Ascending + fn items_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box>> + 'c> { + let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); + let res = match data { + Some(bin) => vec![bin], + None => vec![], + }; + let mapped = res.into_iter().map(|bin| { + let parsed: UniqueRef = from_slice(&bin)?; + Ok((parsed.pk.into_vec(), parsed.value)) + }); + Box::new(mapped) + } } #[cfg(test)] From 06c08584e7f38bc4f24cd735269525d8a4d2a0e4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 12:59:56 +0200 Subject: [PATCH 14/33] Properly query unique indexes --- packages/storage/src/indexed_bucket.rs | 130 ++++++++++++------------- 1 file changed, 62 insertions(+), 68 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 674286065a..59542f36ed 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -35,6 +35,11 @@ where indexes: Vec + 'x>>, } +// 2 main variants: +// * store (namespace, index_name, idx_value, key) -> b"1" - allows many and references pk +// * store (namespace, index_name, idx_value) -> {key, value} - allows one and copies pk and data +// // this would be the primary key - we abstract that too??? +// * store (namespace, index_name, pk) -> value - allows one with data pub trait Index where S: Storage, @@ -60,6 +65,8 @@ where core: &'c Core, idx: &[u8], ) -> Box>> + 'c>; + + // TODO: range over secondary index values? (eg. all results with 30 < age < 40) } /// we pull out Core from IndexedBucket, so we can get a mutable reference to storage @@ -101,28 +108,6 @@ where may_deserialize(&value) } - // THis can be pulled into a trait. - // 1. index: fn(&T) -> Vec - // 2. store: needs access to core for storage and namespace - // - // 2 variants: - // * store (namespace, index_name, idx_value, key) -> b"1" - allows many and references pk - // * store (namespace, index_name, idx_value) -> {key, value} - allows one and copies pk and data - // // this would be the primary key - we abstract that too??? - // * store (namespace, index_name, pk) -> value - allows one with data - - // index is stored (namespace, idx): key -> b"1" - // idx is prefixed and appended to namespace - pub fn add_to_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { - set_with_prefix(self.storage, &self.index_space(index_name, idx), key, b"1"); - } - - // index is stored (namespace, idx): key -> b"1" - // idx is prefixed and appended to namespace - pub fn remove_from_index(&mut self, index_name: &str, idx: &[u8], key: &[u8]) { - remove_with_prefix(self.storage, &self.index_space(index_name, idx), key); - } - fn index_space(&self, index_name: &str, idx: &[u8]) -> Vec { let mut index_space = self.prefix_idx(index_name); let mut key_prefix = to_length_prefixed(idx); @@ -149,32 +134,6 @@ where .map(deserialize_kv::); Box::new(mapped) } - - /// returns all pks that where stored under this secondary index, always Ascending - /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply - pub fn pks_by_index<'c>( - &'c self, - index_name: &str, - idx: &[u8], - ) -> Box> + 'c> { - let start = self.index_space(index_name, idx); - let mapped = - range_with_prefix(self.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); - Box::new(mapped) - } - - /// returns all items that match this secondary index, always by pk Ascending - pub fn items_by_index<'c>( - &'c self, - index_name: &str, - idx: &[u8], - ) -> Box>> + 'c> { - let mapped = self.pks_by_index(index_name, idx).map(move |pk| { - let v = self.load(&pk)?; - Ok((pk, v)) - }); - Box::new(mapped) - } } impl<'a, 'b, 'x, S, T> IndexedBucket<'a, 'b, 'x, S, T> @@ -203,23 +162,53 @@ where indexer: fn(&T) -> Vec, ) -> StdResult { let name = name.into(); + self.can_add_index(&name)?; + + let index = MultiIndex::new(indexer, name); + self.indexes.push(Box::new(index)); + Ok(self) + } + + /// Usage: + /// let mut bucket = IndexedBucket::new(&mut storeage, b"foobar") + /// .with_unique_index("name", |x| x.name.clone())? + /// .with_index("age", by_age)?; + pub fn with_unique_index>( + mut self, + name: U, + indexer: fn(&T) -> Vec, + ) -> StdResult { + let name = name.into(); + self.can_add_index(&name)?; + + let index = UniqueIndex::new(indexer, name); + self.indexes.push(Box::new(index)); + Ok(self) + } + + fn can_add_index(&self, name: &str) -> StdResult<()> { if name.as_bytes() == PREFIX_PK { return Err(StdError::generic_err( "Index _pk is reserved for the primary key", )); } + let dup = self.get_index(name); + match dup { + Some(_) => Err(StdError::generic_err(format!( + "Attempt to write index {} 2 times", + name + ))), + None => Ok(()), + } + } + + fn get_index(&self, name: &str) -> Option<&Box + 'x>> { for existing in self.indexes.iter() { if existing.name() == name { - return Err(StdError::generic_err(format!( - "Attempt to write index {} 2 times", - name - ))); + return Some(existing); } } - - let index = MultiIndex::new(indexer, name); - self.indexes.push(Box::new(index)); - Ok(self) + None } /// save will serialize the model and store, returns an error on serialization issues. @@ -298,13 +287,15 @@ where /// returns all pks that where stored under this secondary index, always Ascending /// this is mainly an internal function, but can be used direcly if you just want to list ids cheaply - /// TODO: return error if index_name is not registered? pub fn pks_by_index<'c>( &'c self, index_name: &str, idx: &[u8], - ) -> Box> + 'c> { - self.core.pks_by_index(index_name, idx) + ) -> StdResult> + 'c>> { + let index = self + .get_index(index_name) + .ok_or_else(|| StdError::not_found(index_name))?; + Ok(index.pks_by_index(&self.core, idx)) } /// returns all items that match this secondary index, always by pk Ascending @@ -312,8 +303,11 @@ where &'c self, index_name: &str, idx: &[u8], - ) -> Box>> + 'c> { - self.core.items_by_index(index_name, idx) + ) -> StdResult>> + 'c>> { + let index = self + .get_index(index_name) + .ok_or_else(|| StdError::not_found(index_name))?; + Ok(index.items_by_index(&self.core, idx)) } } @@ -529,7 +523,7 @@ mod test { let mut bucket = IndexedBucket::new(&mut store, b"data") .with_index("name", by_name) .unwrap() - .with_index("age", by_age) + .with_unique_index("age", by_age) .unwrap(); // save data @@ -545,7 +539,7 @@ mod test { assert_eq!(data, loaded); // load it by secondary index (we must know how to compute this) - let marias: StdResult> = bucket.items_by_index("name", b"Maria").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Maria").unwrap().collect(); let marias = marias.unwrap(); assert_eq!(1, marias.len()); let (k, v) = &marias[0]; @@ -553,26 +547,26 @@ mod test { assert_eq!(&data, v); // other index doesn't match (1 byte after) - let marias: StdResult> = bucket.items_by_index("name", b"Marib").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Marib").unwrap().collect(); assert_eq!(0, marias.unwrap().len()); // other index doesn't match (1 byte before) - let marias: StdResult> = bucket.items_by_index("name", b"Mari`").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Mari`").unwrap().collect(); assert_eq!(0, marias.unwrap().len()); // other index doesn't match (longer) - let marias: StdResult> = bucket.items_by_index("name", b"Maria5").collect(); + let marias: StdResult> = bucket.items_by_index("name", b"Maria5").unwrap().collect(); assert_eq!(0, marias.unwrap().len()); // match on proper age let proper = 42u32.to_be_bytes(); - let marias: StdResult> = bucket.items_by_index("age", &proper).collect(); + let marias: StdResult> = bucket.items_by_index("age", &proper).unwrap().collect(); let marias = marias.unwrap(); assert_eq!(1, marias.len()); // no match on wrong age let too_old = 43u32.to_be_bytes(); - let marias: StdResult> = bucket.items_by_index("age", &too_old).collect(); + let marias: StdResult> = bucket.items_by_index("age", &too_old).unwrap().collect(); assert_eq!(0, marias.unwrap().len()); } } From b913301db08bff50b212a8b94509df88ee4255ed Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 13:06:40 +0200 Subject: [PATCH 15/33] Less alloc on creating indexes --- packages/storage/src/indexed_bucket.rs | 58 +++++++++++--------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 59542f36ed..cb4eedef6e 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -156,14 +156,8 @@ where /// let mut bucket = IndexedBucket::new(&mut storeage, b"foobar") /// .with_unique_index("name", |x| x.name.clone())? /// .with_index("age", by_age)?; - pub fn with_index>( - mut self, - name: U, - indexer: fn(&T) -> Vec, - ) -> StdResult { - let name = name.into(); - self.can_add_index(&name)?; - + pub fn with_index(mut self, name: &'x str, indexer: fn(&T) -> Vec) -> StdResult { + self.can_add_index(name)?; let index = MultiIndex::new(indexer, name); self.indexes.push(Box::new(index)); Ok(self) @@ -173,14 +167,12 @@ where /// let mut bucket = IndexedBucket::new(&mut storeage, b"foobar") /// .with_unique_index("name", |x| x.name.clone())? /// .with_index("age", by_age)?; - pub fn with_unique_index>( + pub fn with_unique_index( mut self, - name: U, + name: &'x str, indexer: fn(&T) -> Vec, ) -> StdResult { - let name = name.into(); - self.can_add_index(&name)?; - + self.can_add_index(name)?; let index = UniqueIndex::new(indexer, name); self.indexes.push(Box::new(index)); Ok(self) @@ -313,37 +305,37 @@ where //------- indexers ------// -pub struct MultiIndex +pub struct MultiIndex<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned + Clone, { idx_fn: fn(&T) -> Vec, - _name: String, + _name: &'a str, _phantom: PhantomData, } -impl MultiIndex +impl<'a, S, T> MultiIndex<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned + Clone, { - pub fn new>(idx_fn: fn(&T) -> Vec, name: U) -> Self { + pub fn new(idx_fn: fn(&T) -> Vec, name: &'a str) -> Self { MultiIndex { idx_fn, - _name: name.into(), + _name: name, _phantom: Default::default(), } } } -impl Index for MultiIndex +impl<'a, S, T> Index for MultiIndex<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned + Clone, { fn name(&self) -> String { - self._name.clone() + self._name.to_string() } fn index(&self, data: &T) -> Vec { @@ -392,43 +384,43 @@ where } } -pub struct UniqueIndex +#[derive(Deserialize, Serialize, Clone)] +pub struct UniqueRef { + pk: Binary, + value: T, +} + +pub struct UniqueIndex<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned + Clone, { idx_fn: fn(&T) -> Vec, - _name: String, + _name: &'a str, _phantom: PhantomData, } -#[derive(Deserialize, Serialize, Clone)] -pub struct UniqueRef { - pk: Binary, - value: T, -} - -impl UniqueIndex +impl<'a, S, T> UniqueIndex<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned + Clone, { - pub fn new>(idx_fn: fn(&T) -> Vec, name: U) -> Self { + pub fn new(idx_fn: fn(&T) -> Vec, name: &'a str) -> Self { UniqueIndex { idx_fn, - _name: name.into(), + _name: name, _phantom: Default::default(), } } } -impl Index for UniqueIndex +impl<'a, S, T> Index for UniqueIndex<'a, S, T> where S: Storage, T: Serialize + DeserializeOwned + Clone, { fn name(&self) -> String { - self._name.clone() + self._name.to_string() } fn index(&self, data: &T) -> Vec { From dfe0616773dbe87f4002ba04e186d03d19c29db3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 13:21:45 +0200 Subject: [PATCH 16/33] Separate index implementation from IndexedBucket --- packages/storage/src/indexed_bucket.rs | 241 +------------------------ packages/storage/src/indexes.rs | 232 ++++++++++++++++++++++++ packages/storage/src/lib.rs | 3 +- 3 files changed, 244 insertions(+), 232 deletions(-) create mode 100644 packages/storage/src/indexes.rs diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index cb4eedef6e..83d5169fce 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -1,22 +1,20 @@ // this module requires iterator to be useful at all #![cfg(feature = "iterator")] -use cosmwasm_std::{from_slice, to_vec, Binary, Order, StdError, StdResult, Storage, KV}; +use cosmwasm_std::{to_vec, Order, StdError, StdResult, Storage, KV}; use serde::de::DeserializeOwned; -// use serde::Serialize; -use serde::{Deserialize, Serialize}; +use serde::Serialize; +use std::marker::PhantomData; +use crate::indexes::{Index, MultiIndex, UniqueIndex}; use crate::namespace_helpers::{ get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, }; use crate::type_helpers::{deserialize_kv, may_deserialize, must_deserialize}; use crate::{to_length_prefixed, to_length_prefixed_nested}; -use serde::export::PhantomData; /// reserved name, no index may register const PREFIX_PK: &[u8] = b"_pk"; -/// MARKER is stored in the multi-index as value, but we only look at the key (which is pk) -const MARKER: &[u8] = b"1"; /// IndexedBucket works like a bucket but has a secondary index /// This is a WIP. @@ -35,49 +33,15 @@ where indexes: Vec + 'x>>, } -// 2 main variants: -// * store (namespace, index_name, idx_value, key) -> b"1" - allows many and references pk -// * store (namespace, index_name, idx_value) -> {key, value} - allows one and copies pk and data -// // this would be the primary key - we abstract that too??? -// * store (namespace, index_name, pk) -> value - allows one with data -pub trait Index -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - // TODO: do we make this any Vec ? - fn name(&self) -> String; - fn index(&self, data: &T) -> Vec; - - fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()>; - fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()>; - - // these should be implemented by all - fn pks_by_index<'c>( - &self, - core: &'c Core, - idx: &[u8], - ) -> Box> + 'c>; - - /// returns all items that match this secondary index, always by pk Ascending - fn items_by_index<'c>( - &self, - core: &'c Core, - idx: &[u8], - ) -> Box>> + 'c>; - - // TODO: range over secondary index values? (eg. all results with 30 < age < 40) -} - /// we pull out Core from IndexedBucket, so we can get a mutable reference to storage /// while holding an immutable iterator over indexers -pub struct Core<'a, 'b, S, T> +pub(crate) struct Core<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned + Clone, { - storage: &'a mut S, - namespace: &'b [u8], + pub storage: &'a mut S, + pub namespace: &'b [u8], _phantom: PhantomData, } @@ -108,18 +72,18 @@ where may_deserialize(&value) } - fn index_space(&self, index_name: &str, idx: &[u8]) -> Vec { + pub fn index_space(&self, index_name: &str, idx: &[u8]) -> Vec { let mut index_space = self.prefix_idx(index_name); let mut key_prefix = to_length_prefixed(idx); index_space.append(&mut key_prefix); index_space } - fn prefix_idx(&self, index_name: &str) -> Vec { + pub fn prefix_idx(&self, index_name: &str) -> Vec { to_length_prefixed_nested(&[self.namespace, index_name.as_bytes()]) } - fn prefix_pk(&self) -> Vec { + pub fn prefix_pk(&self) -> Vec { to_length_prefixed_nested(&[self.namespace, PREFIX_PK]) } @@ -303,191 +267,6 @@ where } } -//------- indexers ------// - -pub struct MultiIndex<'a, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - idx_fn: fn(&T) -> Vec, - _name: &'a str, - _phantom: PhantomData, -} - -impl<'a, S, T> MultiIndex<'a, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - pub fn new(idx_fn: fn(&T) -> Vec, name: &'a str) -> Self { - MultiIndex { - idx_fn, - _name: name, - _phantom: Default::default(), - } - } -} - -impl<'a, S, T> Index for MultiIndex<'a, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - fn name(&self) -> String { - self._name.to_string() - } - - fn index(&self, data: &T) -> Vec { - (self.idx_fn)(data) - } - - fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { - let idx = self.index(data); - set_with_prefix( - core.storage, - &core.index_space(&self._name, &idx), - pk, - MARKER, - ); - Ok(()) - } - - fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()> { - let idx = self.index(old_data); - remove_with_prefix(core.storage, &core.index_space(&self._name, &idx), pk); - Ok(()) - } - - fn pks_by_index<'c>( - &self, - core: &'c Core, - idx: &[u8], - ) -> Box> + 'c> { - let start = core.index_space(&self._name, idx); - let mapped = - range_with_prefix(core.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); - Box::new(mapped) - } - - /// returns all items that match this secondary index, always by pk Ascending - fn items_by_index<'c>( - &self, - core: &'c Core, - idx: &[u8], - ) -> Box>> + 'c> { - let mapped = self.pks_by_index(core, idx).map(move |pk| { - let v = core.load(&pk)?; - Ok((pk, v)) - }); - Box::new(mapped) - } -} - -#[derive(Deserialize, Serialize, Clone)] -pub struct UniqueRef { - pk: Binary, - value: T, -} - -pub struct UniqueIndex<'a, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - idx_fn: fn(&T) -> Vec, - _name: &'a str, - _phantom: PhantomData, -} - -impl<'a, S, T> UniqueIndex<'a, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - pub fn new(idx_fn: fn(&T) -> Vec, name: &'a str) -> Self { - UniqueIndex { - idx_fn, - _name: name, - _phantom: Default::default(), - } - } -} - -impl<'a, S, T> Index for UniqueIndex<'a, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - fn name(&self) -> String { - self._name.to_string() - } - - fn index(&self, data: &T) -> Vec { - (self.idx_fn)(data) - } - - // we store (namespace, index_name, idx_value) -> { pk, value } - fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { - let idx = self.index(data); - let reference = UniqueRef:: { - pk: pk.into(), - value: data.clone(), - }; - set_with_prefix( - core.storage, - &core.prefix_idx(&self._name), - &idx, - &to_vec(&reference)?, - ); - Ok(()) - } - - // we store (namespace, index_name, idx_value) -> { pk, value } - fn remove(&self, core: &mut Core, _pk: &[u8], old_data: &T) -> StdResult<()> { - let idx = self.index(old_data); - remove_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); - Ok(()) - } - - // there is exactly 0 or 1 here... - fn pks_by_index<'c>( - &self, - core: &'c Core, - idx: &[u8], - ) -> Box> + 'c> { - let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); - let res = match data { - Some(bin) => vec![bin], - None => vec![], - }; - let mapped = res.into_iter().map(|bin| { - // TODO: update types so we can return error here - let parsed: UniqueRef = from_slice(&bin).unwrap(); - parsed.pk.into_vec() - }); - Box::new(mapped) - } - - /// returns all items that match this secondary index, always by pk Ascending - fn items_by_index<'c>( - &self, - core: &'c Core, - idx: &[u8], - ) -> Box>> + 'c> { - let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); - let res = match data { - Some(bin) => vec![bin], - None => vec![], - }; - let mapped = res.into_iter().map(|bin| { - let parsed: UniqueRef = from_slice(&bin)?; - Ok((parsed.pk.into_vec(), parsed.value)) - }); - Box::new(mapped) - } -} - #[cfg(test)] mod test { use super::*; diff --git a/packages/storage/src/indexes.rs b/packages/storage/src/indexes.rs new file mode 100644 index 0000000000..97acfe22b4 --- /dev/null +++ b/packages/storage/src/indexes.rs @@ -0,0 +1,232 @@ +// this module requires iterator to be useful at all +#![cfg(feature = "iterator")] + +use cosmwasm_std::{from_slice, to_vec, Binary, Order, StdResult, Storage, KV}; +use serde::de::DeserializeOwned; +use serde::{Deserialize, Serialize}; +use std::marker::PhantomData; + +use crate::indexed_bucket::Core; +use crate::namespace_helpers::{ + get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, +}; + +/// MARKER is stored in the multi-index as value, but we only look at the key (which is pk) +const MARKER: &[u8] = b"1"; + +// 2 main variants: +// * store (namespace, index_name, idx_value, key) -> b"1" - allows many and references pk +// * store (namespace, index_name, idx_value) -> {key, value} - allows one and copies pk and data +// // this would be the primary key - we abstract that too??? +// * store (namespace, index_name, pk) -> value - allows one with data +pub(crate) trait Index +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + // TODO: do we make this any Vec ? + fn name(&self) -> String; + fn index(&self, data: &T) -> Vec; + + fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()>; + fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()>; + + // these should be implemented by all + fn pks_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box> + 'c>; + + /// returns all items that match this secondary index, always by pk Ascending + fn items_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box>> + 'c>; + + // TODO: range over secondary index values? (eg. all results with 30 < age < 40) +} + +pub(crate) struct MultiIndex<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + idx_fn: fn(&T) -> Vec, + _name: &'a str, + _phantom: PhantomData, +} + +impl<'a, S, T> MultiIndex<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + pub fn new(idx_fn: fn(&T) -> Vec, name: &'a str) -> Self { + MultiIndex { + idx_fn, + _name: name, + _phantom: Default::default(), + } + } +} + +impl<'a, S, T> Index for MultiIndex<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + fn name(&self) -> String { + self._name.to_string() + } + + fn index(&self, data: &T) -> Vec { + (self.idx_fn)(data) + } + + fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { + let idx = self.index(data); + set_with_prefix( + core.storage, + &core.index_space(&self._name, &idx), + pk, + MARKER, + ); + Ok(()) + } + + fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()> { + let idx = self.index(old_data); + remove_with_prefix(core.storage, &core.index_space(&self._name, &idx), pk); + Ok(()) + } + + fn pks_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box> + 'c> { + let start = core.index_space(&self._name, idx); + let mapped = + range_with_prefix(core.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); + Box::new(mapped) + } + + /// returns all items that match this secondary index, always by pk Ascending + fn items_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box>> + 'c> { + let mapped = self.pks_by_index(core, idx).map(move |pk| { + let v = core.load(&pk)?; + Ok((pk, v)) + }); + Box::new(mapped) + } +} + +#[derive(Deserialize, Serialize, Clone)] +pub(crate) struct UniqueRef { + pk: Binary, + value: T, +} + +pub(crate) struct UniqueIndex<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + idx_fn: fn(&T) -> Vec, + _name: &'a str, + _phantom: PhantomData, +} + +impl<'a, S, T> UniqueIndex<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + pub fn new(idx_fn: fn(&T) -> Vec, name: &'a str) -> Self { + UniqueIndex { + idx_fn, + _name: name, + _phantom: Default::default(), + } + } +} + +impl<'a, S, T> Index for UniqueIndex<'a, S, T> +where + S: Storage, + T: Serialize + DeserializeOwned + Clone, +{ + fn name(&self) -> String { + self._name.to_string() + } + + fn index(&self, data: &T) -> Vec { + (self.idx_fn)(data) + } + + // we store (namespace, index_name, idx_value) -> { pk, value } + fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { + let idx = self.index(data); + let reference = UniqueRef:: { + pk: pk.into(), + value: data.clone(), + }; + set_with_prefix( + core.storage, + &core.prefix_idx(&self._name), + &idx, + &to_vec(&reference)?, + ); + Ok(()) + } + + // we store (namespace, index_name, idx_value) -> { pk, value } + fn remove(&self, core: &mut Core, _pk: &[u8], old_data: &T) -> StdResult<()> { + let idx = self.index(old_data); + remove_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); + Ok(()) + } + + // there is exactly 0 or 1 here... + fn pks_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box> + 'c> { + let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); + let res = match data { + Some(bin) => vec![bin], + None => vec![], + }; + let mapped = res.into_iter().map(|bin| { + // TODO: update types so we can return error here + let parsed: UniqueRef = from_slice(&bin).unwrap(); + parsed.pk.into_vec() + }); + Box::new(mapped) + } + + /// returns all items that match this secondary index, always by pk Ascending + fn items_by_index<'c>( + &self, + core: &'c Core, + idx: &[u8], + ) -> Box>> + 'c> { + let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); + let res = match data { + Some(bin) => vec![bin], + None => vec![], + }; + let mapped = res.into_iter().map(|bin| { + let parsed: UniqueRef = from_slice(&bin)?; + Ok((parsed.pk.into_vec(), parsed.value)) + }); + Box::new(mapped) + } +} diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index 0394cdf73c..f6e26eff1f 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -1,5 +1,6 @@ mod bucket; mod indexed_bucket; +mod indexes; mod length_prefixed; mod namespace_helpers; mod prefixed_storage; @@ -11,7 +12,7 @@ mod typed; pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; #[cfg(feature = "iterator")] -pub use indexed_bucket::{Core, IndexedBucket, MultiIndex, UniqueIndex}; +pub use indexed_bucket::IndexedBucket; pub use length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; pub use prefixed_storage::{prefixed, prefixed_read, PrefixedStorage, ReadonlyPrefixedStorage}; pub use sequence::{currval, nextval, sequence}; From 3c9ddba90adbccddd42420b66a530ddfac6c777d Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 13:36:11 +0200 Subject: [PATCH 17/33] Remove intermediate steps in IndexedBucket/Core --- packages/storage/src/indexed_bucket.rs | 34 ++++++++++++------------- packages/storage/src/length_prefixed.rs | 17 +++++++++++++ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 83d5169fce..80baa8c4ef 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -7,9 +7,8 @@ use serde::Serialize; use std::marker::PhantomData; use crate::indexes::{Index, MultiIndex, UniqueIndex}; -use crate::namespace_helpers::{ - get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, -}; +use crate::length_prefixed::namespaces_with_key; +use crate::namespace_helpers::range_with_prefix; use crate::type_helpers::{deserialize_kv, may_deserialize, must_deserialize}; use crate::{to_length_prefixed, to_length_prefixed_nested}; @@ -50,25 +49,29 @@ where S: Storage, T: Serialize + DeserializeOwned + Clone, { - pub fn set_pk(&mut self, key: &[u8], updated: &T) -> StdResult<()> { - set_with_prefix(self.storage, &self.prefix_pk(), key, &to_vec(updated)?); + pub fn set_pk(&mut self, pk: &[u8], updated: &T) -> StdResult<()> { + let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); + self.storage.set(&key, &to_vec(updated)?); Ok(()) } - pub fn remove_pk(&mut self, key: &[u8]) { - remove_with_prefix(self.storage, &self.prefix_pk(), key) + pub fn remove_pk(&mut self, pk: &[u8]) { + let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); + self.storage.remove(&key); } /// load will return an error if no data is set at the given key, or on parse error - pub fn load(&self, key: &[u8]) -> StdResult { - let value = get_with_prefix(self.storage, &self.prefix_pk(), key); + pub fn load(&self, pk: &[u8]) -> StdResult { + let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); + let value = self.storage.get(&key); must_deserialize(&value) } /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing - pub fn may_load(&self, key: &[u8]) -> StdResult> { - let value = get_with_prefix(self.storage, &self.prefix_pk(), key); + pub fn may_load(&self, pk: &[u8]) -> StdResult> { + let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); + let value = self.storage.get(&key); may_deserialize(&value) } @@ -83,10 +86,6 @@ where to_length_prefixed_nested(&[self.namespace, index_name.as_bytes()]) } - pub fn prefix_pk(&self) -> Vec { - to_length_prefixed_nested(&[self.namespace, PREFIX_PK]) - } - /// iterates over the items in pk order pub fn range<'c>( &'c self, @@ -94,8 +93,9 @@ where end: Option<&[u8]>, order: Order, ) -> Box>> + 'c> { - let mapped = range_with_prefix(self.storage, &self.prefix_pk(), start, end, order) - .map(deserialize_kv::); + let namespace = to_length_prefixed_nested(&[self.namespace, PREFIX_PK]); + let mapped = + range_with_prefix(self.storage, &namespace, start, end, order).map(deserialize_kv::); Box::new(mapped) } } diff --git a/packages/storage/src/length_prefixed.rs b/packages/storage/src/length_prefixed.rs index 6f6cc7f2d3..e50cbdd7e8 100644 --- a/packages/storage/src/length_prefixed.rs +++ b/packages/storage/src/length_prefixed.rs @@ -29,6 +29,23 @@ pub fn to_length_prefixed_nested(namespaces: &[&[u8]]) -> Vec { out } +/// This is equivalent concat(to_length_prefixed_nested(namespaces), key) +/// But more efficient when the intermediate namespaces often must be recalculated +pub fn namespaces_with_key(namespaces: &[&[u8]], key: &[u8]) -> Vec { + let mut size = key.len(); + for &namespace in namespaces { + size += namespace.len() + 2; + } + + let mut out = Vec::with_capacity(size); + for &namespace in namespaces { + out.extend_from_slice(&encode_length(namespace)); + out.extend_from_slice(namespace); + } + out.extend_from_slice(key); + out +} + /// Encodes the length of a given namespace as a 2 byte big endian encoded integer fn encode_length(namespace: &[u8]) -> [u8; 2] { if namespace.len() > 0xFFFF { From c9bfe678e8ffdfb2537a60a3e847bc510538caf0 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 13:52:16 +0200 Subject: [PATCH 18/33] Move index-specific logic out of Core --- packages/storage/src/indexed_bucket.rs | 13 +------ packages/storage/src/indexes.rs | 52 ++++++++++---------------- 2 files changed, 20 insertions(+), 45 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 80baa8c4ef..5af5c31aac 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -9,8 +9,8 @@ use std::marker::PhantomData; use crate::indexes::{Index, MultiIndex, UniqueIndex}; use crate::length_prefixed::namespaces_with_key; use crate::namespace_helpers::range_with_prefix; +use crate::to_length_prefixed_nested; use crate::type_helpers::{deserialize_kv, may_deserialize, must_deserialize}; -use crate::{to_length_prefixed, to_length_prefixed_nested}; /// reserved name, no index may register const PREFIX_PK: &[u8] = b"_pk"; @@ -75,17 +75,6 @@ where may_deserialize(&value) } - pub fn index_space(&self, index_name: &str, idx: &[u8]) -> Vec { - let mut index_space = self.prefix_idx(index_name); - let mut key_prefix = to_length_prefixed(idx); - index_space.append(&mut key_prefix); - index_space - } - - pub fn prefix_idx(&self, index_name: &str) -> Vec { - to_length_prefixed_nested(&[self.namespace, index_name.as_bytes()]) - } - /// iterates over the items in pk order pub fn range<'c>( &'c self, diff --git a/packages/storage/src/indexes.rs b/packages/storage/src/indexes.rs index 97acfe22b4..14bf8fb2e6 100644 --- a/packages/storage/src/indexes.rs +++ b/packages/storage/src/indexes.rs @@ -7,9 +7,8 @@ use serde::{Deserialize, Serialize}; use std::marker::PhantomData; use crate::indexed_bucket::Core; -use crate::namespace_helpers::{ - get_with_prefix, range_with_prefix, remove_with_prefix, set_with_prefix, -}; +use crate::length_prefixed::{namespaces_with_key, to_length_prefixed_nested}; +use crate::namespace_helpers::range_with_prefix; /// MARKER is stored in the multi-index as value, but we only look at the key (which is pk) const MARKER: &[u8] = b"1"; @@ -87,18 +86,15 @@ where fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { let idx = self.index(data); - set_with_prefix( - core.storage, - &core.index_space(&self._name, &idx), - pk, - MARKER, - ); + let key = namespaces_with_key(&[core.namespace, self._name.as_bytes(), &idx], pk); + core.storage.set(&key, MARKER); Ok(()) } fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()> { let idx = self.index(old_data); - remove_with_prefix(core.storage, &core.index_space(&self._name, &idx), pk); + let key = namespaces_with_key(&[core.namespace, self._name.as_bytes(), &idx], pk); + core.storage.remove(&key); Ok(()) } @@ -107,9 +103,9 @@ where core: &'c Core, idx: &[u8], ) -> Box> + 'c> { - let start = core.index_space(&self._name, idx); - let mapped = - range_with_prefix(core.storage, &start, None, None, Order::Ascending).map(|(k, _)| k); + let namespace = to_length_prefixed_nested(&[core.namespace, self._name.as_bytes(), idx]); + let mapped = range_with_prefix(core.storage, &namespace, None, None, Order::Ascending) + .map(|(k, _)| k); Box::new(mapped) } @@ -173,23 +169,20 @@ where // we store (namespace, index_name, idx_value) -> { pk, value } fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { let idx = self.index(data); + let key = namespaces_with_key(&[core.namespace, self._name.as_bytes()], &idx); let reference = UniqueRef:: { pk: pk.into(), value: data.clone(), }; - set_with_prefix( - core.storage, - &core.prefix_idx(&self._name), - &idx, - &to_vec(&reference)?, - ); + core.storage.set(&key, &to_vec(&reference)?); Ok(()) } // we store (namespace, index_name, idx_value) -> { pk, value } fn remove(&self, core: &mut Core, _pk: &[u8], old_data: &T) -> StdResult<()> { let idx = self.index(old_data); - remove_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); + let key = namespaces_with_key(&[core.namespace, self._name.as_bytes()], &idx); + core.storage.remove(&key); Ok(()) } @@ -199,16 +192,9 @@ where core: &'c Core, idx: &[u8], ) -> Box> + 'c> { - let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); - let res = match data { - Some(bin) => vec![bin], - None => vec![], - }; - let mapped = res.into_iter().map(|bin| { - // TODO: update types so we can return error here - let parsed: UniqueRef = from_slice(&bin).unwrap(); - parsed.pk.into_vec() - }); + // TODO: update types to return StdResult> ? + // should never really happen, but I dislike unwrap + let mapped = self.items_by_index(core, idx).map(|res| res.unwrap().0); Box::new(mapped) } @@ -218,12 +204,12 @@ where core: &'c Core, idx: &[u8], ) -> Box>> + 'c> { - let data = get_with_prefix(core.storage, &core.prefix_idx(&self._name), &idx); - let res = match data { + let key = namespaces_with_key(&[core.namespace, self._name.as_bytes()], idx); + let data = match core.storage.get(&key) { Some(bin) => vec![bin], None => vec![], }; - let mapped = res.into_iter().map(|bin| { + let mapped = data.into_iter().map(|bin| { let parsed: UniqueRef = from_slice(&bin)?; Ok((parsed.pk.into_vec(), parsed.value)) }); From 998739625e605456ccf6db6802f6d03e40716f04 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 13:55:32 +0200 Subject: [PATCH 19/33] Fix clippy warnings --- packages/storage/src/indexed_bucket.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 5af5c31aac..1517651774 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -147,10 +147,10 @@ where } } - fn get_index(&self, name: &str) -> Option<&Box + 'x>> { + fn get_index(&self, name: &str) -> Option<&dyn Index> { for existing in self.indexes.iter() { if existing.name() == name { - return Some(existing); + return Some(existing.as_ref()); } } None From 7cb6f0ba107ba4cdc1d9d4f41aca1d161261e368 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 14:17:49 +0200 Subject: [PATCH 20/33] Fixed clippy for real --- packages/storage/src/length_prefixed.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/storage/src/length_prefixed.rs b/packages/storage/src/length_prefixed.rs index e50cbdd7e8..9c66f80b13 100644 --- a/packages/storage/src/length_prefixed.rs +++ b/packages/storage/src/length_prefixed.rs @@ -31,6 +31,7 @@ pub fn to_length_prefixed_nested(namespaces: &[&[u8]]) -> Vec { /// This is equivalent concat(to_length_prefixed_nested(namespaces), key) /// But more efficient when the intermediate namespaces often must be recalculated +#[allow(dead_code)] pub fn namespaces_with_key(namespaces: &[&[u8]], key: &[u8]) -> Vec { let mut size = key.len(); for &namespace in namespaces { From 021ee9e057b99fe0887e0768115e7e82fd8a0b50 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 17:20:13 +0200 Subject: [PATCH 21/33] Pull out index functions --- packages/storage/src/indexed_bucket.rs | 15 ++++----------- packages/storage/src/indexes.rs | 14 ++++++++++++++ packages/storage/src/lib.rs | 1 + 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 1517651774..8dadfca766 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -260,6 +260,7 @@ where mod test { use super::*; + use crate::indexes::{index_i32, index_string}; use cosmwasm_std::testing::MockStorage; use serde::{Deserialize, Serialize}; @@ -269,21 +270,13 @@ mod test { pub age: i32, } - fn by_name(data: &Data) -> Vec { - data.name.as_bytes().to_vec() - } - - fn by_age(data: &Data) -> Vec { - data.age.to_be_bytes().into() - } - #[test] fn store_and_load_by_index() { let mut store = MockStorage::new(); - let mut bucket = IndexedBucket::new(&mut store, b"data") - .with_index("name", by_name) + let mut bucket = IndexedBucket::<_, Data>::new(&mut store, b"data") + .with_index("name", |d| index_string(&d.name)) .unwrap() - .with_unique_index("age", by_age) + .with_unique_index("age", |d| index_i32(d.age)) .unwrap(); // save data diff --git a/packages/storage/src/indexes.rs b/packages/storage/src/indexes.rs index 14bf8fb2e6..0e97b9d558 100644 --- a/packages/storage/src/indexes.rs +++ b/packages/storage/src/indexes.rs @@ -13,6 +13,20 @@ use crate::namespace_helpers::range_with_prefix; /// MARKER is stored in the multi-index as value, but we only look at the key (which is pk) const MARKER: &[u8] = b"1"; +pub fn index_string(data: &str) -> Vec { + data.as_bytes().to_vec() +} + +// Look at https://docs.rs/endiannezz/0.4.1/endiannezz/trait.Primitive.html +// if you want to make this generic over all ints +pub fn index_u64(data: u64) -> Vec { + data.to_be_bytes().into() +} + +pub fn index_i32(data: i32) -> Vec { + data.to_be_bytes().into() +} + // 2 main variants: // * store (namespace, index_name, idx_value, key) -> b"1" - allows many and references pk // * store (namespace, index_name, idx_value) -> {key, value} - allows one and copies pk and data diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index f6e26eff1f..bcbc5cc163 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -13,6 +13,7 @@ mod typed; pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; #[cfg(feature = "iterator")] pub use indexed_bucket::IndexedBucket; +pub use indexes::{index_i32, index_string, index_u64}; pub use length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; pub use prefixed_storage::{prefixed, prefixed_read, PrefixedStorage, ReadonlyPrefixedStorage}; pub use sequence::{currval, nextval, sequence}; From 7836f6bbd770df3efe324c19d3aaffce1e87dc8f Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 17:49:12 +0200 Subject: [PATCH 22/33] Test and enforce unique index --- packages/storage/src/indexed_bucket.rs | 108 ++++++++++++++++++++++--- packages/storage/src/indexes.rs | 10 ++- packages/storage/src/lib.rs | 1 + 3 files changed, 107 insertions(+), 12 deletions(-) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index 8dadfca766..f80f11f181 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -254,6 +254,21 @@ where .ok_or_else(|| StdError::not_found(index_name))?; Ok(index.items_by_index(&self.core, idx)) } + + // this will return None for 0 items, Some(x) for 1 item, + // and an error for > 1 item. Only meant to be called on unique + // indexes that can return 0 or 1 item + pub fn load_unique_index(&self, index_name: &str, idx: &[u8]) -> StdResult>> { + let mut it = self.items_by_index(index_name, idx)?; + let first = it.next().transpose()?; + match first { + None => Ok(None), + Some(one) => match it.next() { + None => Ok(Some(one)), + Some(_) => Err(StdError::generic_err("Unique Index returned 2 matches")), + }, + } + } } #[cfg(test)] @@ -270,14 +285,18 @@ mod test { pub age: i32, } - #[test] - fn store_and_load_by_index() { - let mut store = MockStorage::new(); - let mut bucket = IndexedBucket::<_, Data>::new(&mut store, b"data") + fn build_bucket(store: &mut S) -> IndexedBucket { + IndexedBucket::::new(store, b"data") .with_index("name", |d| index_string(&d.name)) .unwrap() .with_unique_index("age", |d| index_i32(d.age)) - .unwrap(); + .unwrap() + } + + #[test] + fn store_and_load_by_index() { + let mut store = MockStorage::new(); + let mut bucket = build_bucket(&mut store); // save data let data = Data { @@ -292,7 +311,10 @@ mod test { assert_eq!(data, loaded); // load it by secondary index (we must know how to compute this) - let marias: StdResult> = bucket.items_by_index("name", b"Maria").unwrap().collect(); + let marias: StdResult> = bucket + .items_by_index("name", &index_string("Maria")) + .unwrap() + .collect(); let marias = marias.unwrap(); assert_eq!(1, marias.len()); let (k, v) = &marias[0]; @@ -300,26 +322,90 @@ mod test { assert_eq!(&data, v); // other index doesn't match (1 byte after) - let marias: StdResult> = bucket.items_by_index("name", b"Marib").unwrap().collect(); + let marias: StdResult> = bucket + .items_by_index("name", &index_string("Marib")) + .unwrap() + .collect(); assert_eq!(0, marias.unwrap().len()); // other index doesn't match (1 byte before) - let marias: StdResult> = bucket.items_by_index("name", b"Mari`").unwrap().collect(); + let marias: StdResult> = bucket + .items_by_index("name", &index_string("Mari`")) + .unwrap() + .collect(); assert_eq!(0, marias.unwrap().len()); // other index doesn't match (longer) - let marias: StdResult> = bucket.items_by_index("name", b"Maria5").unwrap().collect(); + let marias: StdResult> = bucket + .items_by_index("name", &index_string("Maria5")) + .unwrap() + .collect(); assert_eq!(0, marias.unwrap().len()); // match on proper age - let proper = 42u32.to_be_bytes(); + let proper = index_i32(42); let marias: StdResult> = bucket.items_by_index("age", &proper).unwrap().collect(); let marias = marias.unwrap(); assert_eq!(1, marias.len()); // no match on wrong age - let too_old = 43u32.to_be_bytes(); + let too_old = index_i32(43); let marias: StdResult> = bucket.items_by_index("age", &too_old).unwrap().collect(); assert_eq!(0, marias.unwrap().len()); } + + #[test] + fn unique_index_enforced() { + let mut store = MockStorage::new(); + let mut bucket = build_bucket(&mut store); + + // first data + let data1 = Data { + name: "Maria".to_string(), + age: 42, + }; + let pk1: &[u8] = b"5627"; + bucket.save(pk1, &data1).unwrap(); + + // same name (multi-index), different age => ok + let data2 = Data { + name: "Maria".to_string(), + age: 23, + }; + let pk2: &[u8] = b"7326"; + bucket.save(pk2, &data2).unwrap(); + + // different name, same age => error + let data3 = Data { + name: "Marta".to_string(), + age: 42, + }; + let pk3: &[u8] = b"8263"; + // enforce this returns some error + bucket.save(pk3, &data3).unwrap_err(); + + // query by unique key + // match on proper age + let age42 = index_i32(42); + let (k, v) = bucket.load_unique_index("age", &age42).unwrap().unwrap(); + assert_eq!(k.as_slice(), pk1); + assert_eq!(&v.name, "Maria"); + assert_eq!(v.age, 42); + + // match on other age + let age23 = index_i32(23); + let (k, v) = bucket.load_unique_index("age", &age23).unwrap().unwrap(); + assert_eq!(k.as_slice(), pk2); + assert_eq!(&v.name, "Maria"); + assert_eq!(v.age, 23); + + // if we delete the first one, we can add the blocked one + bucket.remove(pk1).unwrap(); + bucket.save(pk3, &data3).unwrap(); + // now 42 is the new owner + let (k, v) = bucket.load_unique_index("age", &age42).unwrap().unwrap(); + assert_eq!(k.as_slice(), pk3); + assert_eq!(&v.name, "Marta"); + assert_eq!(v.age, 42); + } } diff --git a/packages/storage/src/indexes.rs b/packages/storage/src/indexes.rs index 0e97b9d558..ed0daed606 100644 --- a/packages/storage/src/indexes.rs +++ b/packages/storage/src/indexes.rs @@ -1,7 +1,7 @@ // this module requires iterator to be useful at all #![cfg(feature = "iterator")] -use cosmwasm_std::{from_slice, to_vec, Binary, Order, StdResult, Storage, KV}; +use cosmwasm_std::{from_slice, to_vec, Binary, Order, StdError, StdResult, Storage, KV}; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use std::marker::PhantomData; @@ -184,6 +184,14 @@ where fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { let idx = self.index(data); let key = namespaces_with_key(&[core.namespace, self._name.as_bytes()], &idx); + // error if this is already set + if core.storage.get(&key).is_some() { + return Err(StdError::generic_err(format!( + "Violates unique constraint on index `{}`", + self._name + ))); + } + let reference = UniqueRef:: { pk: pk.into(), value: data.clone(), diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index bcbc5cc163..479e660ef8 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -13,6 +13,7 @@ mod typed; pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; #[cfg(feature = "iterator")] pub use indexed_bucket::IndexedBucket; +#[cfg(feature = "iterator")] pub use indexes::{index_i32, index_string, index_u64}; pub use length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; pub use prefixed_storage::{prefixed, prefixed_read, PrefixedStorage, ReadonlyPrefixedStorage}; From 355dfa1998668e57f3f772245ab34f3434c6e0b5 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 17:59:06 +0200 Subject: [PATCH 23/33] Ensure multi-index properly updated --- packages/storage/src/indexed_bucket.rs | 56 ++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index f80f11f181..e362c6bd89 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -277,6 +277,7 @@ mod test { use crate::indexes::{index_i32, index_string}; use cosmwasm_std::testing::MockStorage; + use cosmwasm_std::MemoryStorage; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, PartialEq, Debug, Clone)] @@ -408,4 +409,59 @@ mod test { assert_eq!(&v.name, "Marta"); assert_eq!(v.age, 42); } + + #[test] + fn remove_and_update_reflected_on_indexes() { + let mut store = MockStorage::new(); + let mut bucket = build_bucket(&mut store); + + let name_count = |bucket: &IndexedBucket, name: &str| -> usize { + bucket + .items_by_index("name", &index_string(name)) + .unwrap() + .count() + }; + + // set up some data + let data1 = Data { + name: "John".to_string(), + age: 22, + }; + let pk1: &[u8] = b"john"; + bucket.save(pk1, &data1).unwrap(); + let data2 = Data { + name: "John".to_string(), + age: 25, + }; + let pk2: &[u8] = b"john2"; + bucket.save(pk2, &data2).unwrap(); + let data3 = Data { + name: "Fred".to_string(), + age: 33, + }; + let pk3: &[u8] = b"fred"; + bucket.save(pk3, &data3).unwrap(); + + // find 2 Johns, 1 Fred, and no Mary + assert_eq!(name_count(&bucket, "John"), 2); + assert_eq!(name_count(&bucket, "Fred"), 1); + assert_eq!(name_count(&bucket, "Mary"), 0); + + // remove john 2 + bucket.remove(pk2).unwrap(); + // change fred to mary + bucket + .update(pk3, |d| -> StdResult<_> { + let mut x = d.unwrap(); + assert_eq!(&x.name, "Fred"); + x.name = "Mary".to_string(); + Ok(x) + }) + .unwrap(); + + // find 1 Johns, no Fred, and 1 Mary + assert_eq!(name_count(&bucket, "John"), 1); + assert_eq!(name_count(&bucket, "Fred"), 0); + assert_eq!(name_count(&bucket, "Mary"), 1); + } } From a36cda51d67d01ca64ec85a54d76eed9fd455044 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 14:14:37 +0200 Subject: [PATCH 24/33] Update Bucket to be more compatible with Core --- packages/storage/src/bucket.rs | 56 ++++++++++++++++--------- packages/storage/src/length_prefixed.rs | 25 +++++++++++ 2 files changed, 62 insertions(+), 19 deletions(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index b1d0c957d5..1ae843250b 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -5,16 +5,18 @@ use cosmwasm_std::{to_vec, ReadonlyStorage, StdError, StdResult, Storage}; #[cfg(feature = "iterator")] use cosmwasm_std::{Order, KV}; -use crate::length_prefixed::{to_length_prefixed, to_length_prefixed_nested}; +use crate::length_prefixed::{to_length_prefixed, to_length_prefixed_nested, nested_namespaces_with_key}; #[cfg(feature = "iterator")] use crate::namespace_helpers::range_with_prefix; -use crate::namespace_helpers::{get_with_prefix, remove_with_prefix, set_with_prefix}; +use crate::namespace_helpers::get_with_prefix; #[cfg(feature = "iterator")] use crate::type_helpers::deserialize_kv; use crate::type_helpers::{may_deserialize, must_deserialize}; +pub(crate) const PREFIX_PK: &[u8] = b"_pk"; + /// An alias of Bucket::new for less verbose usage -pub fn bucket<'a, S, T>(storage: &'a mut S, namespace: &[u8]) -> Bucket<'a, S, T> +pub fn bucket<'a, 'b, S, T>(storage: &'a mut S, namespace: &'b [u8]) -> Bucket<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned, @@ -31,69 +33,85 @@ where ReadonlyBucket::new(storage, namespace) } -pub struct Bucket<'a, S, T> +/// Bucket stores all data under a series of length-prefixed steps: (namespaces..., "_pk"). +/// After this is created (each step length-prefixed), we just append the bucket key at the end. +/// +/// The reason for the "_pk" at the end is to allow easy extensibility with IndexedBuckets, which +/// can also store indexes under the same namespace +pub struct Bucket<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned, { - storage: &'a mut S, - prefix: Vec, + pub(crate) storage: &'a mut S, + pub(crate) namespaces: Vec<&'b[u8]>, // see https://doc.rust-lang.org/std/marker/struct.PhantomData.html#unused-type-parameters for why this is needed data: PhantomData, } -impl<'a, S, T> Bucket<'a, S, T> +impl<'a, 'b, S, T> Bucket<'a, 'b, S, T> where S: Storage, T: Serialize + DeserializeOwned, { - pub fn new(storage: &'a mut S, namespace: &[u8]) -> Self { + pub fn new(storage: &'a mut S, namespace: &'b [u8]) -> Self { Bucket { storage, - prefix: to_length_prefixed(namespace), + namespaces: vec![namespace], data: PhantomData, } } - pub fn multilevel(storage: &'a mut S, namespaces: &[&[u8]]) -> Self { + pub fn multilevel(storage: &'a mut S, namespace: &[&'b[u8]]) -> Self { Bucket { storage, - prefix: to_length_prefixed_nested(namespaces), + namespaces: namespace.to_vec(), data: PhantomData, } } + /// This provides the raw storage key that we use to access a given "bucket key". + /// Calling this with `key = b""` will give us the pk prefix for range queries + pub fn build_prefixed_key(&self, key: &[u8]) -> Vec { + nested_namespaces_with_key(&self.namespaces, &[PREFIX_PK], key) + } + /// save will serialize the model and store, returns an error on serialization issues pub fn save(&mut self, key: &[u8], data: &T) -> StdResult<()> { - set_with_prefix(self.storage, &self.prefix, key, &to_vec(data)?); + let key = self.build_prefixed_key(key); + self.storage.set(&key, &to_vec(data)?); Ok(()) } pub fn remove(&mut self, key: &[u8]) { - remove_with_prefix(self.storage, &self.prefix, key) + let key = self.build_prefixed_key(key); + self.storage.remove(&key); } /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { - let value = get_with_prefix(self.storage, &self.prefix, key); + let key = self.build_prefixed_key(key); + let value = self.storage.get(&key); must_deserialize(&value) } /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing pub fn may_load(&self, key: &[u8]) -> StdResult> { - let value = get_with_prefix(self.storage, &self.prefix, key); + let key = self.build_prefixed_key(key); + let value = self.storage.get(&key); may_deserialize(&value) } #[cfg(feature = "iterator")] - pub fn range<'b>( - &'b self, + pub fn range<'c>( + &'c self, start: Option<&[u8]>, end: Option<&[u8]>, order: Order, - ) -> Box>> + 'b> { - let mapped = range_with_prefix(self.storage, &self.prefix, start, end, order) + ) -> Box>> + 'c> { + let namespace = self.build_prefixed_key(b""); + let mapped = range_with_prefix(self.storage, &namespace, start, end, order) .map(deserialize_kv::); Box::new(mapped) } diff --git a/packages/storage/src/length_prefixed.rs b/packages/storage/src/length_prefixed.rs index 9c66f80b13..08171a86df 100644 --- a/packages/storage/src/length_prefixed.rs +++ b/packages/storage/src/length_prefixed.rs @@ -47,6 +47,31 @@ pub fn namespaces_with_key(namespaces: &[&[u8]], key: &[u8]) -> Vec { out } +/// Customization of namespaces_with_key for when +/// there are multiple sets we do not want to combine just to call this +pub fn nested_namespaces_with_key(top_names: &[&[u8]], sub_names: &[&[u8]], key: &[u8]) -> Vec { + let mut size = key.len(); + for &namespace in top_names { + size += namespace.len() + 2; + } + for &namespace in sub_names { + size += namespace.len() + 2; + } + + let mut out = Vec::with_capacity(size); + for &namespace in top_names { + out.extend_from_slice(&encode_length(namespace)); + out.extend_from_slice(namespace); + } + for &namespace in sub_names { + out.extend_from_slice(&encode_length(namespace)); + out.extend_from_slice(namespace); + } + out.extend_from_slice(key); + out +} + + /// Encodes the length of a given namespace as a 2 byte big endian encoded integer fn encode_length(namespace: &[u8]) -> [u8; 2] { if namespace.len() > 0xFFFF { From 120921f793837813dfce75a662b6213eaf8ec7a2 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 16:46:20 +0200 Subject: [PATCH 25/33] Bucket and ReadOnlyBucket pass tests --- packages/storage/src/bucket.rs | 57 +++++++++++++++---------- packages/storage/src/length_prefixed.rs | 1 - 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index 1ae843250b..7554b15bf9 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -5,10 +5,9 @@ use cosmwasm_std::{to_vec, ReadonlyStorage, StdError, StdResult, Storage}; #[cfg(feature = "iterator")] use cosmwasm_std::{Order, KV}; -use crate::length_prefixed::{to_length_prefixed, to_length_prefixed_nested, nested_namespaces_with_key}; +use crate::length_prefixed::nested_namespaces_with_key; #[cfg(feature = "iterator")] use crate::namespace_helpers::range_with_prefix; -use crate::namespace_helpers::get_with_prefix; #[cfg(feature = "iterator")] use crate::type_helpers::deserialize_kv; use crate::type_helpers::{may_deserialize, must_deserialize}; @@ -25,7 +24,10 @@ where } /// An alias of ReadonlyBucket::new for less verbose usage -pub fn bucket_read<'a, S, T>(storage: &'a S, namespace: &[u8]) -> ReadonlyBucket<'a, S, T> +pub fn bucket_read<'a, 'b, S, T>( + storage: &'a S, + namespace: &'b [u8], +) -> ReadonlyBucket<'a, 'b, S, T> where S: ReadonlyStorage, T: Serialize + DeserializeOwned, @@ -44,7 +46,7 @@ where T: Serialize + DeserializeOwned, { pub(crate) storage: &'a mut S, - pub(crate) namespaces: Vec<&'b[u8]>, + pub(crate) namespaces: Vec<&'b [u8]>, // see https://doc.rust-lang.org/std/marker/struct.PhantomData.html#unused-type-parameters for why this is needed data: PhantomData, } @@ -62,10 +64,10 @@ where } } - pub fn multilevel(storage: &'a mut S, namespace: &[&'b[u8]]) -> Self { + pub fn multilevel(storage: &'a mut S, namespaces: &[&'b [u8]]) -> Self { Bucket { storage, - namespaces: namespace.to_vec(), + namespaces: namespaces.to_vec(), data: PhantomData, } } @@ -111,8 +113,8 @@ where order: Order, ) -> Box>> + 'c> { let namespace = self.build_prefixed_key(b""); - let mapped = range_with_prefix(self.storage, &namespace, start, end, order) - .map(deserialize_kv::); + let mapped = + range_with_prefix(self.storage, &namespace, start, end, order).map(deserialize_kv::); Box::new(mapped) } @@ -132,60 +134,69 @@ where } } -pub struct ReadonlyBucket<'a, S, T> +pub struct ReadonlyBucket<'a, 'b, S, T> where S: ReadonlyStorage, T: Serialize + DeserializeOwned, { - storage: &'a S, - prefix: Vec, + pub(crate) storage: &'a S, + pub(crate) namespaces: Vec<&'b [u8]>, // see https://doc.rust-lang.org/std/marker/struct.PhantomData.html#unused-type-parameters for why this is needed data: PhantomData, } -impl<'a, S, T> ReadonlyBucket<'a, S, T> +impl<'a, 'b, S, T> ReadonlyBucket<'a, 'b, S, T> where S: ReadonlyStorage, T: Serialize + DeserializeOwned, { - pub fn new(storage: &'a S, namespace: &[u8]) -> Self { + pub fn new(storage: &'a S, namespace: &'b [u8]) -> Self { ReadonlyBucket { storage, - prefix: to_length_prefixed(namespace), + namespaces: vec![namespace], data: PhantomData, } } - pub fn multilevel(storage: &'a S, namespaces: &[&[u8]]) -> Self { + pub fn multilevel(storage: &'a S, namespaces: &[&'b [u8]]) -> Self { ReadonlyBucket { storage, - prefix: to_length_prefixed_nested(namespaces), + namespaces: namespaces.to_vec(), data: PhantomData, } } + /// This provides the raw storage key that we use to access a given "bucket key". + /// Calling this with `key = b""` will give us the pk prefix for range queries + pub fn build_prefixed_key(&self, key: &[u8]) -> Vec { + nested_namespaces_with_key(&self.namespaces, &[PREFIX_PK], key) + } + /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { - let value = get_with_prefix(self.storage, &self.prefix, key); + let key = self.build_prefixed_key(key); + let value = self.storage.get(&key); must_deserialize(&value) } /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing pub fn may_load(&self, key: &[u8]) -> StdResult> { - let value = get_with_prefix(self.storage, &self.prefix, key); + let key = self.build_prefixed_key(key); + let value = self.storage.get(&key); may_deserialize(&value) } #[cfg(feature = "iterator")] - pub fn range<'b>( - &'b self, + pub fn range<'c>( + &'c self, start: Option<&[u8]>, end: Option<&[u8]>, order: Order, - ) -> Box>> + 'b> { - let mapped = range_with_prefix(self.storage, &self.prefix, start, end, order) - .map(deserialize_kv::); + ) -> Box>> + 'c> { + let namespace = self.build_prefixed_key(b""); + let mapped = + range_with_prefix(self.storage, &namespace, start, end, order).map(deserialize_kv::); Box::new(mapped) } } diff --git a/packages/storage/src/length_prefixed.rs b/packages/storage/src/length_prefixed.rs index 08171a86df..f693dc1837 100644 --- a/packages/storage/src/length_prefixed.rs +++ b/packages/storage/src/length_prefixed.rs @@ -71,7 +71,6 @@ pub fn nested_namespaces_with_key(top_names: &[&[u8]], sub_names: &[&[u8]], key: out } - /// Encodes the length of a given namespace as a 2 byte big endian encoded integer fn encode_length(namespace: &[u8]) -> [u8; 2] { if namespace.len() > 0xFFFF { From 569c788c81dd2d07f6506fb16d1ebc83a26ffb98 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 3 Oct 2020 17:07:53 +0200 Subject: [PATCH 26/33] IndexedBucket and Indexes use Bucket not Core --- packages/storage/src/bucket.rs | 18 +++-- packages/storage/src/indexed_bucket.rs | 91 ++++---------------------- packages/storage/src/indexes.rs | 59 ++++++++--------- 3 files changed, 54 insertions(+), 114 deletions(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index 7554b15bf9..8416fa8cb2 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -74,25 +74,31 @@ where /// This provides the raw storage key that we use to access a given "bucket key". /// Calling this with `key = b""` will give us the pk prefix for range queries - pub fn build_prefixed_key(&self, key: &[u8]) -> Vec { + pub fn build_primary_key(&self, key: &[u8]) -> Vec { nested_namespaces_with_key(&self.namespaces, &[PREFIX_PK], key) } + /// This provides the raw storage key that we use to access a secondary index + /// Calling this with `key = b""` will give us the index prefix for range queries + pub fn build_secondary_key(&self, path: &[&[u8]], key: &[u8]) -> Vec { + nested_namespaces_with_key(&self.namespaces, path, key) + } + /// save will serialize the model and store, returns an error on serialization issues pub fn save(&mut self, key: &[u8], data: &T) -> StdResult<()> { - let key = self.build_prefixed_key(key); + let key = self.build_primary_key(key); self.storage.set(&key, &to_vec(data)?); Ok(()) } pub fn remove(&mut self, key: &[u8]) { - let key = self.build_prefixed_key(key); + let key = self.build_primary_key(key); self.storage.remove(&key); } /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { - let key = self.build_prefixed_key(key); + let key = self.build_primary_key(key); let value = self.storage.get(&key); must_deserialize(&value) } @@ -100,7 +106,7 @@ where /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing pub fn may_load(&self, key: &[u8]) -> StdResult> { - let key = self.build_prefixed_key(key); + let key = self.build_primary_key(key); let value = self.storage.get(&key); may_deserialize(&value) } @@ -112,7 +118,7 @@ where end: Option<&[u8]>, order: Order, ) -> Box>> + 'c> { - let namespace = self.build_prefixed_key(b""); + let namespace = self.build_primary_key(b""); let mapped = range_with_prefix(self.storage, &namespace, start, end, order).map(deserialize_kv::); Box::new(mapped) diff --git a/packages/storage/src/indexed_bucket.rs b/packages/storage/src/indexed_bucket.rs index e362c6bd89..32c550f0a9 100644 --- a/packages/storage/src/indexed_bucket.rs +++ b/packages/storage/src/indexed_bucket.rs @@ -1,16 +1,12 @@ // this module requires iterator to be useful at all #![cfg(feature = "iterator")] -use cosmwasm_std::{to_vec, Order, StdError, StdResult, Storage, KV}; +use cosmwasm_std::{Order, StdError, StdResult, Storage, KV}; use serde::de::DeserializeOwned; use serde::Serialize; -use std::marker::PhantomData; +use crate::bucket::Bucket; use crate::indexes::{Index, MultiIndex, UniqueIndex}; -use crate::length_prefixed::namespaces_with_key; -use crate::namespace_helpers::range_with_prefix; -use crate::to_length_prefixed_nested; -use crate::type_helpers::{deserialize_kv, may_deserialize, must_deserialize}; /// reserved name, no index may register const PREFIX_PK: &[u8] = b"_pk"; @@ -28,67 +24,10 @@ where S: Storage + 'x, T: Serialize + DeserializeOwned + Clone + 'x, { - core: Core<'a, 'b, S, T>, + bucket: Bucket<'a, 'b, S, T>, indexes: Vec + 'x>>, } -/// we pull out Core from IndexedBucket, so we can get a mutable reference to storage -/// while holding an immutable iterator over indexers -pub(crate) struct Core<'a, 'b, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - pub storage: &'a mut S, - pub namespace: &'b [u8], - _phantom: PhantomData, -} - -impl<'a, 'b, S, T> Core<'a, 'b, S, T> -where - S: Storage, - T: Serialize + DeserializeOwned + Clone, -{ - pub fn set_pk(&mut self, pk: &[u8], updated: &T) -> StdResult<()> { - let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); - self.storage.set(&key, &to_vec(updated)?); - Ok(()) - } - - pub fn remove_pk(&mut self, pk: &[u8]) { - let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); - self.storage.remove(&key); - } - - /// load will return an error if no data is set at the given key, or on parse error - pub fn load(&self, pk: &[u8]) -> StdResult { - let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); - let value = self.storage.get(&key); - must_deserialize(&value) - } - - /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. - /// returns an error on issues parsing - pub fn may_load(&self, pk: &[u8]) -> StdResult> { - let key = namespaces_with_key(&[self.namespace, PREFIX_PK], pk); - let value = self.storage.get(&key); - may_deserialize(&value) - } - - /// iterates over the items in pk order - pub fn range<'c>( - &'c self, - start: Option<&[u8]>, - end: Option<&[u8]>, - order: Order, - ) -> Box>> + 'c> { - let namespace = to_length_prefixed_nested(&[self.namespace, PREFIX_PK]); - let mapped = - range_with_prefix(self.storage, &namespace, start, end, order).map(deserialize_kv::); - Box::new(mapped) - } -} - impl<'a, 'b, 'x, S, T> IndexedBucket<'a, 'b, 'x, S, T> where S: Storage, @@ -96,11 +35,7 @@ where { pub fn new(storage: &'a mut S, namespace: &'b [u8]) -> Self { IndexedBucket { - core: Core { - storage, - namespace, - _phantom: Default::default(), - }, + bucket: Bucket::new(storage, namespace), indexes: vec![], } } @@ -176,16 +111,16 @@ where if let Some(old) = old_data { // Note: this didn't work as we cannot mutably borrow self (remove_from_index) inside the iterator for index in self.indexes.iter() { - index.remove(&mut self.core, key, old)?; + index.remove(&mut self.bucket, key, old)?; } } if let Some(updated) = data { for index in self.indexes.iter() { - index.insert(&mut self.core, key, updated)?; + index.insert(&mut self.bucket, key, updated)?; } - self.core.set_pk(key, updated)?; + self.bucket.save(key, updated)?; } else { - self.core.remove_pk(key); + self.bucket.remove(key); } Ok(()) } @@ -211,13 +146,13 @@ where /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { - self.core.load(key) + self.bucket.load(key) } /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing pub fn may_load(&self, key: &[u8]) -> StdResult> { - self.core.may_load(key) + self.bucket.may_load(key) } /// iterates over the items in pk order @@ -227,7 +162,7 @@ where end: Option<&[u8]>, order: Order, ) -> Box>> + 'c> { - self.core.range(start, end, order) + self.bucket.range(start, end, order) } /// returns all pks that where stored under this secondary index, always Ascending @@ -240,7 +175,7 @@ where let index = self .get_index(index_name) .ok_or_else(|| StdError::not_found(index_name))?; - Ok(index.pks_by_index(&self.core, idx)) + Ok(index.pks_by_index(&self.bucket, idx)) } /// returns all items that match this secondary index, always by pk Ascending @@ -252,7 +187,7 @@ where let index = self .get_index(index_name) .ok_or_else(|| StdError::not_found(index_name))?; - Ok(index.items_by_index(&self.core, idx)) + Ok(index.items_by_index(&self.bucket, idx)) } // this will return None for 0 items, Some(x) for 1 item, diff --git a/packages/storage/src/indexes.rs b/packages/storage/src/indexes.rs index ed0daed606..236f3c80b7 100644 --- a/packages/storage/src/indexes.rs +++ b/packages/storage/src/indexes.rs @@ -6,9 +6,8 @@ use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use std::marker::PhantomData; -use crate::indexed_bucket::Core; -use crate::length_prefixed::{namespaces_with_key, to_length_prefixed_nested}; use crate::namespace_helpers::range_with_prefix; +use crate::Bucket; /// MARKER is stored in the multi-index as value, but we only look at the key (which is pk) const MARKER: &[u8] = b"1"; @@ -41,20 +40,20 @@ where fn name(&self) -> String; fn index(&self, data: &T) -> Vec; - fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()>; - fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()>; + fn insert(&self, bucket: &mut Bucket, pk: &[u8], data: &T) -> StdResult<()>; + fn remove(&self, bucket: &mut Bucket, pk: &[u8], old_data: &T) -> StdResult<()>; // these should be implemented by all fn pks_by_index<'c>( &self, - core: &'c Core, + bucket: &'c Bucket, idx: &[u8], ) -> Box> + 'c>; /// returns all items that match this secondary index, always by pk Ascending fn items_by_index<'c>( &self, - core: &'c Core, + bucket: &'c Bucket, idx: &[u8], ) -> Box>> + 'c>; @@ -98,27 +97,27 @@ where (self.idx_fn)(data) } - fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { + fn insert(&self, bucket: &mut Bucket, pk: &[u8], data: &T) -> StdResult<()> { let idx = self.index(data); - let key = namespaces_with_key(&[core.namespace, self._name.as_bytes(), &idx], pk); - core.storage.set(&key, MARKER); + let key = bucket.build_secondary_key(&[self._name.as_bytes(), &idx], pk); + bucket.storage.set(&key, MARKER); Ok(()) } - fn remove(&self, core: &mut Core, pk: &[u8], old_data: &T) -> StdResult<()> { + fn remove(&self, bucket: &mut Bucket, pk: &[u8], old_data: &T) -> StdResult<()> { let idx = self.index(old_data); - let key = namespaces_with_key(&[core.namespace, self._name.as_bytes(), &idx], pk); - core.storage.remove(&key); + let key = bucket.build_secondary_key(&[self._name.as_bytes(), &idx], pk); + bucket.storage.remove(&key); Ok(()) } fn pks_by_index<'c>( &self, - core: &'c Core, + bucket: &'c Bucket, idx: &[u8], ) -> Box> + 'c> { - let namespace = to_length_prefixed_nested(&[core.namespace, self._name.as_bytes(), idx]); - let mapped = range_with_prefix(core.storage, &namespace, None, None, Order::Ascending) + let namespace = bucket.build_secondary_key(&[self._name.as_bytes(), &idx], b""); + let mapped = range_with_prefix(bucket.storage, &namespace, None, None, Order::Ascending) .map(|(k, _)| k); Box::new(mapped) } @@ -126,11 +125,11 @@ where /// returns all items that match this secondary index, always by pk Ascending fn items_by_index<'c>( &self, - core: &'c Core, + bucket: &'c Bucket, idx: &[u8], ) -> Box>> + 'c> { - let mapped = self.pks_by_index(core, idx).map(move |pk| { - let v = core.load(&pk)?; + let mapped = self.pks_by_index(bucket, idx).map(move |pk| { + let v = bucket.load(&pk)?; Ok((pk, v)) }); Box::new(mapped) @@ -181,11 +180,11 @@ where } // we store (namespace, index_name, idx_value) -> { pk, value } - fn insert(&self, core: &mut Core, pk: &[u8], data: &T) -> StdResult<()> { + fn insert(&self, bucket: &mut Bucket, pk: &[u8], data: &T) -> StdResult<()> { let idx = self.index(data); - let key = namespaces_with_key(&[core.namespace, self._name.as_bytes()], &idx); + let key = bucket.build_secondary_key(&[self._name.as_bytes()], &idx); // error if this is already set - if core.storage.get(&key).is_some() { + if bucket.storage.get(&key).is_some() { return Err(StdError::generic_err(format!( "Violates unique constraint on index `{}`", self._name @@ -196,38 +195,38 @@ where pk: pk.into(), value: data.clone(), }; - core.storage.set(&key, &to_vec(&reference)?); + bucket.storage.set(&key, &to_vec(&reference)?); Ok(()) } // we store (namespace, index_name, idx_value) -> { pk, value } - fn remove(&self, core: &mut Core, _pk: &[u8], old_data: &T) -> StdResult<()> { + fn remove(&self, bucket: &mut Bucket, _pk: &[u8], old_data: &T) -> StdResult<()> { let idx = self.index(old_data); - let key = namespaces_with_key(&[core.namespace, self._name.as_bytes()], &idx); - core.storage.remove(&key); + let key = bucket.build_secondary_key(&[self._name.as_bytes()], &idx); + bucket.storage.remove(&key); Ok(()) } // there is exactly 0 or 1 here... fn pks_by_index<'c>( &self, - core: &'c Core, + bucket: &'c Bucket, idx: &[u8], ) -> Box> + 'c> { // TODO: update types to return StdResult> ? // should never really happen, but I dislike unwrap - let mapped = self.items_by_index(core, idx).map(|res| res.unwrap().0); + let mapped = self.items_by_index(bucket, idx).map(|res| res.unwrap().0); Box::new(mapped) } /// returns all items that match this secondary index, always by pk Ascending fn items_by_index<'c>( &self, - core: &'c Core, + bucket: &'c Bucket, idx: &[u8], ) -> Box>> + 'c> { - let key = namespaces_with_key(&[core.namespace, self._name.as_bytes()], idx); - let data = match core.storage.get(&key) { + let key = bucket.build_secondary_key(&[self._name.as_bytes()], &idx); + let data = match bucket.storage.get(&key) { Some(bin) => vec![bin], None => vec![], }; From 68f6805e9f3b4dce7710df3f48eb0cae4df44526 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 4 Oct 2020 00:01:42 +0200 Subject: [PATCH 27/33] Remove multilevel constructors --- packages/storage/src/bucket.rs | 60 ++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index 8416fa8cb2..d6283016c4 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -5,7 +5,7 @@ use cosmwasm_std::{to_vec, ReadonlyStorage, StdError, StdResult, Storage}; #[cfg(feature = "iterator")] use cosmwasm_std::{Order, KV}; -use crate::length_prefixed::nested_namespaces_with_key; +use crate::length_prefixed::{nested_namespaces_with_key, namespaces_with_key}; #[cfg(feature = "iterator")] use crate::namespace_helpers::range_with_prefix; #[cfg(feature = "iterator")] @@ -35,7 +35,7 @@ where ReadonlyBucket::new(storage, namespace) } -/// Bucket stores all data under a series of length-prefixed steps: (namespaces..., "_pk"). +/// Bucket stores all data under a series of length-prefixed steps: (namespace, "_pk"). /// After this is created (each step length-prefixed), we just append the bucket key at the end. /// /// The reason for the "_pk" at the end is to allow easy extensibility with IndexedBuckets, which @@ -46,7 +46,7 @@ where T: Serialize + DeserializeOwned, { pub(crate) storage: &'a mut S, - pub(crate) namespaces: Vec<&'b [u8]>, + pub namespace: &'b [u8], // see https://doc.rust-lang.org/std/marker/struct.PhantomData.html#unused-type-parameters for why this is needed data: PhantomData, } @@ -56,18 +56,22 @@ where S: Storage, T: Serialize + DeserializeOwned, { + /// After some reflection, I removed multilevel, as what I really wanted was a composite primary key. + /// For eg. allowances, the multilevel design would make it ("bucket", owner, "_pk", spender) + /// and then maybe ("bucket", owner, "expires", expires_idx, pk) as a secondary index. This is NOT what we want. + /// + /// What we want is ("bucket", "_pk", owner, spender) for the first key. And + /// ("bucket", "expires", expires_idx, pk) as the secondary index. This is not done with "multi-level" + /// but by supporting CompositeKeys. Looking something like: + /// + /// `Bucket::new(storage, "bucket).save(&(owner, spender).pk(), &allowance)` + /// + /// Need to figure out the most ergonomic approach for the composite keys, but it should + /// live outside the Bucket, and just convert into a normal `&[u8]` for this array. pub fn new(storage: &'a mut S, namespace: &'b [u8]) -> Self { Bucket { storage, - namespaces: vec![namespace], - data: PhantomData, - } - } - - pub fn multilevel(storage: &'a mut S, namespaces: &[&'b [u8]]) -> Self { - Bucket { - storage, - namespaces: namespaces.to_vec(), + namespace, data: PhantomData, } } @@ -75,13 +79,13 @@ where /// This provides the raw storage key that we use to access a given "bucket key". /// Calling this with `key = b""` will give us the pk prefix for range queries pub fn build_primary_key(&self, key: &[u8]) -> Vec { - nested_namespaces_with_key(&self.namespaces, &[PREFIX_PK], key) + namespaces_with_key(&[&self.namespace, PREFIX_PK], key) } /// This provides the raw storage key that we use to access a secondary index /// Calling this with `key = b""` will give us the index prefix for range queries pub fn build_secondary_key(&self, path: &[&[u8]], key: &[u8]) -> Vec { - nested_namespaces_with_key(&self.namespaces, path, key) + nested_namespaces_with_key(&[self.namespace], path, key) } /// save will serialize the model and store, returns an error on serialization issues @@ -146,7 +150,7 @@ where T: Serialize + DeserializeOwned, { pub(crate) storage: &'a S, - pub(crate) namespaces: Vec<&'b [u8]>, + pub(crate) namespace: &'b [u8], // see https://doc.rust-lang.org/std/marker/struct.PhantomData.html#unused-type-parameters for why this is needed data: PhantomData, } @@ -159,28 +163,26 @@ where pub fn new(storage: &'a S, namespace: &'b [u8]) -> Self { ReadonlyBucket { storage, - namespaces: vec![namespace], - data: PhantomData, - } - } - - pub fn multilevel(storage: &'a S, namespaces: &[&'b [u8]]) -> Self { - ReadonlyBucket { - storage, - namespaces: namespaces.to_vec(), + namespace, data: PhantomData, } } /// This provides the raw storage key that we use to access a given "bucket key". /// Calling this with `key = b""` will give us the pk prefix for range queries - pub fn build_prefixed_key(&self, key: &[u8]) -> Vec { - nested_namespaces_with_key(&self.namespaces, &[PREFIX_PK], key) + pub fn build_primary_key(&self, key: &[u8]) -> Vec { + namespaces_with_key(&[&self.namespace, PREFIX_PK], key) + } + + /// This provides the raw storage key that we use to access a secondary index + /// Calling this with `key = b""` will give us the index prefix for range queries + pub fn build_secondary_key(&self, path: &[&[u8]], key: &[u8]) -> Vec { + nested_namespaces_with_key(&[self.namespace], path, key) } /// load will return an error if no data is set at the given key, or on parse error pub fn load(&self, key: &[u8]) -> StdResult { - let key = self.build_prefixed_key(key); + let key = self.build_primary_key(key); let value = self.storage.get(&key); must_deserialize(&value) } @@ -188,7 +190,7 @@ where /// may_load will parse the data stored at the key if present, returns Ok(None) if no data there. /// returns an error on issues parsing pub fn may_load(&self, key: &[u8]) -> StdResult> { - let key = self.build_prefixed_key(key); + let key = self.build_primary_key(key); let value = self.storage.get(&key); may_deserialize(&value) } @@ -200,7 +202,7 @@ where end: Option<&[u8]>, order: Order, ) -> Box>> + 'c> { - let namespace = self.build_prefixed_key(b""); + let namespace = self.build_primary_key(b""); let mapped = range_with_prefix(self.storage, &namespace, start, end, order).map(deserialize_kv::); Box::new(mapped) From 166253b316197238dc33bffb3348f87dbf91e329 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 4 Oct 2020 00:22:39 +0200 Subject: [PATCH 28/33] Add Pk2 and Pk3 as examples of composite keys --- packages/storage/src/bucket.rs | 64 ++++++++++++++++++++++++- packages/storage/src/length_prefixed.rs | 14 ++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index d6283016c4..210a01cb86 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -5,7 +5,9 @@ use cosmwasm_std::{to_vec, ReadonlyStorage, StdError, StdResult, Storage}; #[cfg(feature = "iterator")] use cosmwasm_std::{Order, KV}; -use crate::length_prefixed::{nested_namespaces_with_key, namespaces_with_key}; +use crate::length_prefixed::{ + decode_length, length_prefixed_with_key, namespaces_with_key, nested_namespaces_with_key, +}; #[cfg(feature = "iterator")] use crate::namespace_helpers::range_with_prefix; #[cfg(feature = "iterator")] @@ -35,6 +37,51 @@ where ReadonlyBucket::new(storage, namespace) } +//--- TODO: sort this out better ----// + +pub trait PrimaryKey { + fn pk(&self) -> Vec; + fn parse(data: Vec) -> Self; +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Pk2(Vec, Vec); + +impl PrimaryKey for Pk2 { + fn pk(&self) -> Vec { + length_prefixed_with_key(&self.0, &self.1) + } + + fn parse(pk: Vec) -> Self { + // TODO: is there a way to do this without reallocating memory? + // I think I read something about that + let l = decode_length(&pk[..2]); + Pk2(pk[2..l + 2].to_vec(), pk[l + 2..].to_vec()) + } +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Pk3(Vec, Vec, Vec); + +impl PrimaryKey for Pk3 { + fn pk(&self) -> Vec { + namespaces_with_key(&[&self.0, &self.1], &self.2) + } + + fn parse(pk: Vec) -> Self { + // TODO: is there a way to do this without reallocating memory? + // I think I read something about that + let l = decode_length(&pk[..2]); + let l2 = decode_length(&pk[l + 2..l + 4]); + let first = pk[2..l + 2].to_vec(); + let second = pk[l + 4..l + l2 + 4].to_vec(); + let third = pk[l + l2 + 4..].to_vec(); + Pk3(first, second, third) + } +} + +//---- END TODO -------// + /// Bucket stores all data under a series of length-prefixed steps: (namespace, "_pk"). /// After this is created (each step length-prefixed), we just append the bucket key at the end. /// @@ -222,6 +269,21 @@ mod test { pub age: i32, } + #[test] + fn composite_keys() { + let composite = Pk2(b"its".to_vec(), b"windy".to_vec()); + let key = composite.pk(); + assert_eq!(10, key.len()); + let parsed = Pk2::parse(key); + assert_eq!(parsed, composite); + + let composite = Pk3(b"winters".to_vec(), b"really".to_vec(), b"windy".to_vec()); + let key = composite.pk(); + assert_eq!(22, key.len()); + let parsed = Pk3::parse(key); + assert_eq!(parsed, composite); + } + #[test] fn store_and_load() { let mut store = MockStorage::new(); diff --git a/packages/storage/src/length_prefixed.rs b/packages/storage/src/length_prefixed.rs index f693dc1837..bc8528ca4e 100644 --- a/packages/storage/src/length_prefixed.rs +++ b/packages/storage/src/length_prefixed.rs @@ -29,6 +29,14 @@ pub fn to_length_prefixed_nested(namespaces: &[&[u8]]) -> Vec { out } +pub fn length_prefixed_with_key(namespace: &[u8], key: &[u8]) -> Vec { + let mut out = Vec::with_capacity(namespace.len() + 2 + key.len()); + out.extend_from_slice(&encode_length(namespace)); + out.extend_from_slice(namespace); + out.extend_from_slice(key); + out +} + /// This is equivalent concat(to_length_prefixed_nested(namespaces), key) /// But more efficient when the intermediate namespaces often must be recalculated #[allow(dead_code)] @@ -80,6 +88,12 @@ fn encode_length(namespace: &[u8]) -> [u8; 2] { [length_bytes[2], length_bytes[3]] } +// pub(crate) fn decode_length(prefix: [u8; 2]) -> usize { +pub(crate) fn decode_length(prefix: &[u8]) -> usize { + // TODO: enforce exactly 2 bytes somehow, but usable with slices + (prefix[0] as usize) * 256 + (prefix[1] as usize) +} + #[cfg(test)] mod test { use super::*; From 2ac9a950135a27388b56bf9b94c939668f2bfc2f Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 4 Oct 2020 00:39:16 +0200 Subject: [PATCH 29/33] Cleanup Pks a bit, add range mapper --- packages/storage/src/bucket.rs | 41 ++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index 210a01cb86..0f26498e5f 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -41,7 +41,15 @@ where pub trait PrimaryKey { fn pk(&self) -> Vec; - fn parse(data: Vec) -> Self; + fn parse(data: &[u8]) -> Self; + + fn from_kv(kv: (Vec, T)) -> (Self, T) + where + Self: std::marker::Sized, + { + let (k, v) = kv; + (Self::parse(&k), v) + } } #[derive(Debug, Clone, PartialEq)] @@ -52,9 +60,7 @@ impl PrimaryKey for Pk2 { length_prefixed_with_key(&self.0, &self.1) } - fn parse(pk: Vec) -> Self { - // TODO: is there a way to do this without reallocating memory? - // I think I read something about that + fn parse(pk: &[u8]) -> Self { let l = decode_length(&pk[..2]); Pk2(pk[2..l + 2].to_vec(), pk[l + 2..].to_vec()) } @@ -68,9 +74,7 @@ impl PrimaryKey for Pk3 { namespaces_with_key(&[&self.0, &self.1], &self.2) } - fn parse(pk: Vec) -> Self { - // TODO: is there a way to do this without reallocating memory? - // I think I read something about that + fn parse(pk: &[u8]) -> Self { let l = decode_length(&pk[..2]); let l2 = decode_length(&pk[l + 2..l + 4]); let first = pk[2..l + 2].to_vec(); @@ -274,16 +278,35 @@ mod test { let composite = Pk2(b"its".to_vec(), b"windy".to_vec()); let key = composite.pk(); assert_eq!(10, key.len()); - let parsed = Pk2::parse(key); + let parsed = Pk2::parse(&key); assert_eq!(parsed, composite); let composite = Pk3(b"winters".to_vec(), b"really".to_vec(), b"windy".to_vec()); let key = composite.pk(); assert_eq!(22, key.len()); - let parsed = Pk3::parse(key); + let parsed = Pk3::parse(&key); assert_eq!(parsed, composite); } + #[test] + fn composite_keys_parsing() { + // Try from a KV (as if we got a range) + let john = Data { + name: "John".to_string(), + age: 123, + }; + let composite = Pk3(b"lots".to_vec(), b"of".to_vec(), b"text".to_vec()); + + // demo usage as if we mapped over a range iterator + let mut it = vec![(composite.pk(), john.clone())] + .into_iter() + .map(Pk3::from_kv); + let (k1, v1) = it.next().unwrap(); + assert_eq!(k1, composite); + assert_eq!(v1, john); + assert!(it.next().is_none()); + } + #[test] fn store_and_load() { let mut store = MockStorage::new(); From cb80d068c73f0a3a39c6e9eb531d28a0ce5afcd4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 4 Oct 2020 00:46:27 +0200 Subject: [PATCH 30/33] Prototype of range_prefixed for scanning part of a composite index --- packages/storage/src/bucket.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index 0f26498e5f..821a111edf 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -179,6 +179,26 @@ where Box::new(mapped) } + // TODO: test this, just an idea now, need more work on Pks + // Also, how much do we trim from the keys? Leave just the last part of the PK, right? + + /// This lets us grab all items under the beginning of a composite key. + /// If we store under `Pk2(owner, spender)`, then we pass `prefixes: &[owner]` here + /// To list all spenders under the owner + #[cfg(feature = "iterator")] + pub fn range_prefixed<'c>( + &'c self, + prefixes: &[&[u8]], + start: Option<&[u8]>, + end: Option<&[u8]>, + order: Order, + ) -> Box>> + 'c> { + let namespace = nested_namespaces_with_key(&[&self.namespace, PREFIX_PK], prefixes, b""); + let mapped = + range_with_prefix(self.storage, &namespace, start, end, order).map(deserialize_kv::); + Box::new(mapped) + } + /// Loads the data, perform the specified action, and store the result /// in the database. This is shorthand for some common sequences, which may be useful. /// From e1ac2c18e09e81cc0caad795a9f70b06599b7c76 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 4 Oct 2020 12:59:40 +0200 Subject: [PATCH 31/33] Less allocations in PrimaryKeys --- packages/storage/src/bucket.rs | 59 +++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index 821a111edf..51e8226eaf 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -40,10 +40,19 @@ where //--- TODO: sort this out better ----// pub trait PrimaryKey { + type Output; + fn pk(&self) -> Vec; - fn parse(data: &[u8]) -> Self; + fn parse(data: &[u8]) -> Self::Output; + + // convert a PK into an owned variant (Vec rather than &[u8]) + // this can be done brute force, but please override for a cheaper version + // FIXME: better name for this function - to_owned() sounded good, but uses Cloned. Other ideas? + fn to_output(&self) -> Self::Output { + Self::parse(&self.pk()) + } - fn from_kv(kv: (Vec, T)) -> (Self, T) + fn from_kv(kv: (Vec, T)) -> (Self::Output, T) where Self: std::marker::Sized, { @@ -53,34 +62,48 @@ pub trait PrimaryKey { } #[derive(Debug, Clone, PartialEq)] -pub struct Pk2(Vec, Vec); +pub struct Pk2<'a>(pub &'a [u8], pub &'a [u8]); + +impl<'a> PrimaryKey for Pk2<'a> { + type Output = (Vec, Vec); -impl PrimaryKey for Pk2 { fn pk(&self) -> Vec { - length_prefixed_with_key(&self.0, &self.1) + length_prefixed_with_key(self.0, self.1) + } + + fn to_output(&self) -> Self::Output { + (self.0.to_vec(), self.1.to_vec()) } - fn parse(pk: &[u8]) -> Self { + fn parse(pk: &[u8]) -> Self::Output { let l = decode_length(&pk[..2]); - Pk2(pk[2..l + 2].to_vec(), pk[l + 2..].to_vec()) + let first = pk[2..l + 2].to_vec(); + let second = pk[l + 2..].to_vec(); + (first, second) } } #[derive(Debug, Clone, PartialEq)] -pub struct Pk3(Vec, Vec, Vec); +pub struct Pk3<'a>(&'a [u8], &'a [u8], &'a [u8]); + +impl<'a> PrimaryKey for Pk3<'a> { + type Output = (Vec, Vec, Vec); -impl PrimaryKey for Pk3 { fn pk(&self) -> Vec { - namespaces_with_key(&[&self.0, &self.1], &self.2) + namespaces_with_key(&[self.0, self.1], self.2) + } + + fn to_output(&self) -> Self::Output { + (self.0.to_vec(), self.1.to_vec(), self.2.to_vec()) } - fn parse(pk: &[u8]) -> Self { + fn parse(pk: &[u8]) -> Self::Output { let l = decode_length(&pk[..2]); let l2 = decode_length(&pk[l + 2..l + 4]); let first = pk[2..l + 2].to_vec(); let second = pk[l + 4..l + l2 + 4].to_vec(); let third = pk[l + l2 + 4..].to_vec(); - Pk3(first, second, third) + (first, second, third) } } @@ -295,17 +318,17 @@ mod test { #[test] fn composite_keys() { - let composite = Pk2(b"its".to_vec(), b"windy".to_vec()); + let composite = Pk2(b"its", b"windy"); let key = composite.pk(); assert_eq!(10, key.len()); let parsed = Pk2::parse(&key); - assert_eq!(parsed, composite); + assert_eq!(parsed, composite.to_output()); - let composite = Pk3(b"winters".to_vec(), b"really".to_vec(), b"windy".to_vec()); + let composite = Pk3(b"winters", b"really", b"windy"); let key = composite.pk(); assert_eq!(22, key.len()); let parsed = Pk3::parse(&key); - assert_eq!(parsed, composite); + assert_eq!(parsed, composite.to_output()); } #[test] @@ -315,14 +338,14 @@ mod test { name: "John".to_string(), age: 123, }; - let composite = Pk3(b"lots".to_vec(), b"of".to_vec(), b"text".to_vec()); + let composite = Pk3(b"lots", b"of", b"text"); // demo usage as if we mapped over a range iterator let mut it = vec![(composite.pk(), john.clone())] .into_iter() .map(Pk3::from_kv); let (k1, v1) = it.next().unwrap(); - assert_eq!(k1, composite); + assert_eq!(k1, composite.to_output()); assert_eq!(v1, john); assert!(it.next().is_none()); } From 7e74d254694b97ec1f141bf508fb04a64268be50 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 4 Oct 2020 14:41:58 +0200 Subject: [PATCH 32/33] Test usage with Pk2 and minor cleanup --- packages/storage/src/bucket.rs | 252 +++++++++++++++++++-------------- packages/storage/src/lib.rs | 2 +- 2 files changed, 147 insertions(+), 107 deletions(-) diff --git a/packages/storage/src/bucket.rs b/packages/storage/src/bucket.rs index 51e8226eaf..5aa16a6773 100644 --- a/packages/storage/src/bucket.rs +++ b/packages/storage/src/bucket.rs @@ -37,78 +37,6 @@ where ReadonlyBucket::new(storage, namespace) } -//--- TODO: sort this out better ----// - -pub trait PrimaryKey { - type Output; - - fn pk(&self) -> Vec; - fn parse(data: &[u8]) -> Self::Output; - - // convert a PK into an owned variant (Vec rather than &[u8]) - // this can be done brute force, but please override for a cheaper version - // FIXME: better name for this function - to_owned() sounded good, but uses Cloned. Other ideas? - fn to_output(&self) -> Self::Output { - Self::parse(&self.pk()) - } - - fn from_kv(kv: (Vec, T)) -> (Self::Output, T) - where - Self: std::marker::Sized, - { - let (k, v) = kv; - (Self::parse(&k), v) - } -} - -#[derive(Debug, Clone, PartialEq)] -pub struct Pk2<'a>(pub &'a [u8], pub &'a [u8]); - -impl<'a> PrimaryKey for Pk2<'a> { - type Output = (Vec, Vec); - - fn pk(&self) -> Vec { - length_prefixed_with_key(self.0, self.1) - } - - fn to_output(&self) -> Self::Output { - (self.0.to_vec(), self.1.to_vec()) - } - - fn parse(pk: &[u8]) -> Self::Output { - let l = decode_length(&pk[..2]); - let first = pk[2..l + 2].to_vec(); - let second = pk[l + 2..].to_vec(); - (first, second) - } -} - -#[derive(Debug, Clone, PartialEq)] -pub struct Pk3<'a>(&'a [u8], &'a [u8], &'a [u8]); - -impl<'a> PrimaryKey for Pk3<'a> { - type Output = (Vec, Vec, Vec); - - fn pk(&self) -> Vec { - namespaces_with_key(&[self.0, self.1], self.2) - } - - fn to_output(&self) -> Self::Output { - (self.0.to_vec(), self.1.to_vec(), self.2.to_vec()) - } - - fn parse(pk: &[u8]) -> Self::Output { - let l = decode_length(&pk[..2]); - let l2 = decode_length(&pk[l + 2..l + 4]); - let first = pk[2..l + 2].to_vec(); - let second = pk[l + 4..l + l2 + 4].to_vec(); - let third = pk[l + l2 + 4..].to_vec(); - (first, second, third) - } -} - -//---- END TODO -------// - /// Bucket stores all data under a series of length-prefixed steps: (namespace, "_pk"). /// After this is created (each step length-prefixed), we just append the bucket key at the end. /// @@ -238,6 +166,74 @@ where } } +pub trait PrimaryKey { + type Output; + + fn pk(&self) -> Vec; + fn parse(data: &[u8]) -> Self::Output; + + // convert a PK into an owned variant (Vec rather than &[u8]) + // this can be done brute force, but please override for a cheaper version + // FIXME: better name for this function - to_owned() sounded good, but uses Cloned. Other ideas? + fn to_output(&self) -> Self::Output { + Self::parse(&self.pk()) + } + + fn from_kv(kv: (Vec, T)) -> (Self::Output, T) + where + Self: std::marker::Sized, + { + let (k, v) = kv; + (Self::parse(&k), v) + } +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Pk2<'a>(pub &'a [u8], pub &'a [u8]); + +impl<'a> PrimaryKey for Pk2<'a> { + type Output = (Vec, Vec); + + fn pk(&self) -> Vec { + length_prefixed_with_key(self.0, self.1) + } + + fn to_output(&self) -> Self::Output { + (self.0.to_vec(), self.1.to_vec()) + } + + fn parse(pk: &[u8]) -> Self::Output { + let l = decode_length(&pk[..2]); + let first = pk[2..l + 2].to_vec(); + let second = pk[l + 2..].to_vec(); + (first, second) + } +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Pk3<'a>(&'a [u8], &'a [u8], &'a [u8]); + +impl<'a> PrimaryKey for Pk3<'a> { + type Output = (Vec, Vec, Vec); + + fn pk(&self) -> Vec { + namespaces_with_key(&[self.0, self.1], self.2) + } + + fn to_output(&self) -> Self::Output { + (self.0.to_vec(), self.1.to_vec(), self.2.to_vec()) + } + + fn parse(pk: &[u8]) -> Self::Output { + let l = decode_length(&pk[..2]); + let l2 = decode_length(&pk[l + 2..l + 4]); + let first = pk[2..l + 2].to_vec(); + let second = pk[l + 4..l + l2 + 4].to_vec(); + let third = pk[l + l2 + 4..].to_vec(); + (first, second, third) + } +} + pub struct ReadonlyBucket<'a, 'b, S, T> where S: ReadonlyStorage, @@ -316,40 +312,6 @@ mod test { pub age: i32, } - #[test] - fn composite_keys() { - let composite = Pk2(b"its", b"windy"); - let key = composite.pk(); - assert_eq!(10, key.len()); - let parsed = Pk2::parse(&key); - assert_eq!(parsed, composite.to_output()); - - let composite = Pk3(b"winters", b"really", b"windy"); - let key = composite.pk(); - assert_eq!(22, key.len()); - let parsed = Pk3::parse(&key); - assert_eq!(parsed, composite.to_output()); - } - - #[test] - fn composite_keys_parsing() { - // Try from a KV (as if we got a range) - let john = Data { - name: "John".to_string(), - age: 123, - }; - let composite = Pk3(b"lots", b"of", b"text"); - - // demo usage as if we mapped over a range iterator - let mut it = vec![(composite.pk(), john.clone())] - .into_iter() - .map(Pk3::from_kv); - let (k1, v1) = it.next().unwrap(); - assert_eq!(k1, composite.to_output()); - assert_eq!(v1, john); - assert!(it.next().is_none()); - } - #[test] fn store_and_load() { let mut store = MockStorage::new(); @@ -628,4 +590,82 @@ mod test { assert_eq!(data[0], (b"jose".to_vec(), jose)); assert_eq!(data[1], (b"maria".to_vec(), maria)); } + + #[test] + fn composite_keys() { + let composite = Pk2(b"its", b"windy"); + let key = composite.pk(); + assert_eq!(10, key.len()); + let parsed = Pk2::parse(&key); + assert_eq!(parsed, composite.to_output()); + + let composite = Pk3(b"winters", b"really", b"windy"); + let key = composite.pk(); + assert_eq!(22, key.len()); + let parsed = Pk3::parse(&key); + assert_eq!(parsed, composite.to_output()); + } + + #[test] + fn composite_keys_parsing() { + // Try from a KV (as if we got a range) + let john = Data { + name: "John".to_string(), + age: 123, + }; + let composite = Pk3(b"lots", b"of", b"text"); + + // demo usage as if we mapped over a range iterator + let mut it = vec![(composite.pk(), john.clone())] + .into_iter() + .map(Pk3::from_kv); + let (k1, v1) = it.next().unwrap(); + assert_eq!(k1, composite.to_output()); + assert_eq!(v1, john); + assert!(it.next().is_none()); + } + + #[test] + #[cfg(features = "iterator")] + fn bucket_with_composite_pk() { + let mut store = MockStorage::new(); + let mut bucket = Bucket::<_, 64>::new(&mut store, b"allowance"); + + let owner1: &[u8] = b"john"; + let owner2: &[u8] = b"juan"; + let spender1: &[u8] = b"marco"; + let spender2: &[u8] = b"maria"; + let spender3: &[u8] = b"martian"; + + // store some data with composite key + bucket.save(&Pk2(owner1, spender1).pk(), &100).unwrap(); + bucket.save(&Pk2(owner1, spender2).pk(), &250).unwrap(); + bucket.save(&Pk2(owner2, spender1).pk(), &77).unwrap(); + bucket.save(&Pk2(owner2, spender3).pk(), &444).unwrap(); + + // query by full key + assert_eq!(100, bucket.load(&Pk2(owner1, spender1).pk()).unwrap()); + assert_eq!(444, bucket.load(&Pk2(owner2, spender3).pk()).unwrap()); + + // range over one owner. since it is prefixed, we only get the remaining part of the pk (spender) + let spenders: StdResult> = bucket + .range_prefixed(&[owner1], None, None, Order::Ascending) + .collect(); + let spenders = spenders.unwrap(); + assert_eq!(2, spenders.len()); + assert_eq!(spenders[0], (spender1.to_vec(), 100)); + assert_eq!(spenders[1], (spender2.to_vec(), 250)); + + // range over all data. use Pk2::from_kv to parse out the composite key (owner, spender) + let spenders: StdResult> = bucket + .range(None, None, Order::Ascending) + .map(Pk2::from_kv) + .collect(); + let spenders = spenders.unwrap(); + assert_eq!(4, spenders.len()); + assert_eq!(spenders[0], (Pk2(owner1, spender1).to_output(), 100)); + assert_eq!(spenders[1], (Pk2(owner1, spender2).to_output(), 250)); + assert_eq!(spenders[2], (Pk2(owner2, spender1).to_output(), 77)); + assert_eq!(spenders[3], (Pk2(owner2, spender3).to_output(), 444)); + } } diff --git a/packages/storage/src/lib.rs b/packages/storage/src/lib.rs index 479e660ef8..7276e6eb8d 100644 --- a/packages/storage/src/lib.rs +++ b/packages/storage/src/lib.rs @@ -10,7 +10,7 @@ mod transactions; mod type_helpers; mod typed; -pub use bucket::{bucket, bucket_read, Bucket, ReadonlyBucket}; +pub use bucket::{bucket, bucket_read, Bucket, Pk2, Pk3, PrimaryKey, ReadonlyBucket}; #[cfg(feature = "iterator")] pub use indexed_bucket::IndexedBucket; #[cfg(feature = "iterator")] From 89a6c1ff6e0562a730454ece3058ede28b520f31 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 4 Oct 2020 14:55:25 +0200 Subject: [PATCH 33/33] Update CHANGELOG and MIGRATING --- CHANGELOG.md | 8 ++++++++ MIGRATING.md | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f96c92b7f8..2dc9fa769c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,14 @@ namespace in `PrefixedStorage::new`, `PrefixedStorage::multilevel`, `ReadonlyPrefixedStorage::new`, `ReadonlyPrefixedStorage::multilevel`, `prefixed` and `prefixed_read`. +- Remove `Bucket::multilevel` as it was a misnomer. Provide support for + composite `PrimaryKey`s in storage, using `Pk2` and `Pk3` for 2 or 3 items + keys. Add `Bucket::range_prefixed` to provide the first section(s) of the + composite key and iterate under that. +- Bucket stores all data under `(namespace, "_pk", key)`, where the first two + are length-prefixed. This is a different layout from previous form + where it was `(namespace, key)` and this is intended to support co-existence + with `IndexedBucket`. (It only changes the raw storage layout, APIs don't change). **cosmwasm-vm** diff --git a/MIGRATING.md b/MIGRATING.md index 8e105915f8..9e869b8895 100644 --- a/MIGRATING.md +++ b/MIGRATING.md @@ -162,6 +162,52 @@ major releases of `cosmwasm`. Note that you can also view the }) ``` +- Remove `Bukcet::multilevel` constructor. The bucket now has exactly one namespace + and if we need `multilevel`, it was really to provide composite primary keys. + We now do that explicitly in a way that allows better compatibility + with `IndexedBucket`. The test `bucket_with_composite_pk` in `bucket.rs` is + a good (tested) example of the new API. + + ```rust + // before (from cw20-base) + pub fn allowances<'a, S: Storage>( + storage: &'a mut S, + owner: &CanonicalAddr, + ) -> Bucket<'a, S, AllowanceResponse> { + Bucket::multilevel(&[PREFIX_ALLOWANCE, owner.as_slice()], storage) + } + + allowances(&mut deps.storage, owner_raw).update(spender_raw.as_slice(), |allow| { + // code omitted... + })?; + + let allowances: StdResult> = allowances_read(&deps.storage, &owner_raw) + .range(start.as_deref(), None, Order::Ascending) + .take(limit) + .map(|item| { + // code omitted... + }) + .collect(); + + // after + pub fn allowances(storage: &mut S) -> Bucket { + Bucket::new(PREFIX_ALLOWANCE, storage) + } + + allowances(&mut deps.storage).update(&Pk2(owner_raw.as_slice(), spender_raw.as_slice()).pk(), |allow| { + // code omitted... + })?; + + let allowances: StdResult> = allowances(&deps.storage) + .range_prefixed(owner_raw.as_slice(), start.as_deref(), None, Order::Ascending) + .take(limit) + .map(|item| { + // code omitted... + }) + .collect(); + ``` + + ## 0.9 -> 0.10 Integration tests: