-
Notifications
You must be signed in to change notification settings - Fork 482
Simple Mapping type improvements #979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 37 commits
70172ac
0ce829f
e7e7f5d
1fead2f
62a6f01
f25b560
9622ad5
73d4ab9
92a4ce6
e1650c2
60c3370
cfd4b03
60bd5cd
e173a4d
99cad62
047ae35
b880b5a
20395ab
955896b
de7e67f
623ba34
9d12813
387b752
5ce455c
e272f3d
b0ecb48
ebfaea5
e652165
91f65f1
8ee0882
8fcdbdf
89f5799
53e63b4
cf823d1
674a04d
46286d4
8da218f
9ee7ee6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,188 @@ | ||||||||
| // Copyright 2018-2021 Parity Technologies (UK) Ltd. | ||||||||
| // | ||||||||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||
| // you may not use this file except in compliance with the License. | ||||||||
| // You may obtain a copy of the License at | ||||||||
| // | ||||||||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||||||||
| // | ||||||||
| // Unless required by applicable law or agreed to in writing, software | ||||||||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||||||||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||
| // See the License for the specific language governing permissions and | ||||||||
| // limitations under the License. | ||||||||
|
|
||||||||
| //! A simple mapping to contract storage. | ||||||||
| //! | ||||||||
| //! # Note | ||||||||
| //! | ||||||||
| //! This mapping doesn't actually "own" any data. | ||||||||
| //! Instead it is just a simple wrapper around the contract storage facilities. | ||||||||
| use crate::traits::{ | ||||||||
| pull_packed_root_opt, | ||||||||
| push_packed_root, | ||||||||
| ExtKeyPtr, | ||||||||
| KeyPtr, | ||||||||
| PackedLayout, | ||||||||
| SpreadAllocate, | ||||||||
| SpreadLayout, | ||||||||
| }; | ||||||||
| use core::marker::PhantomData; | ||||||||
| use ink_env::hash::{ | ||||||||
| Blake2x256, | ||||||||
| HashOutput, | ||||||||
| }; | ||||||||
| use ink_primitives::Key; | ||||||||
|
|
||||||||
| /// A mapping of key-value pairs directly into contract storage. | ||||||||
| #[cfg_attr(feature = "std", derive(scale_info::TypeInfo))] | ||||||||
| #[derive(Default)] | ||||||||
| pub struct Mapping<K, V> { | ||||||||
| offset_key: Key, | ||||||||
| _marker: PhantomData<fn() -> (K, V)>, | ||||||||
| } | ||||||||
|
|
||||||||
| impl<K, V> core::fmt::Debug for Mapping<K, V> { | ||||||||
| fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { | ||||||||
| f.debug_struct("Mapping") | ||||||||
| .field("offset_key", &self.offset_key) | ||||||||
| .finish() | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| impl<K, V> Mapping<K, V> { | ||||||||
| /// Creates a new empty `Mapping`. | ||||||||
| fn new(offset_key: Key) -> Self { | ||||||||
| Self { | ||||||||
| offset_key, | ||||||||
| _marker: Default::default(), | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| impl<K, V> Mapping<K, V> | ||||||||
| where | ||||||||
| K: PackedLayout, | ||||||||
| V: PackedLayout, | ||||||||
| { | ||||||||
| /// Insert the given `value` to the contract storage. | ||||||||
| #[inline] | ||||||||
| pub fn insert<Q, R>(&mut self, key: Q, value: &R) | ||||||||
| where | ||||||||
| Q: scale::EncodeLike<K>, | ||||||||
| R: scale::EncodeLike<V> + PackedLayout, | ||||||||
| { | ||||||||
| push_packed_root(value, &self.storage_key(key)); | ||||||||
| } | ||||||||
|
|
||||||||
| /// Get the `value` at `key` from the contract storage. | ||||||||
| /// | ||||||||
| /// Returns `None` if no `value` exists at the given `key`. | ||||||||
| #[inline] | ||||||||
| pub fn get<Q>(&self, key: Q) -> Option<V> | ||||||||
| where | ||||||||
| Q: scale::EncodeLike<K>, | ||||||||
| { | ||||||||
| pull_packed_root_opt(&self.storage_key(key)) | ||||||||
| } | ||||||||
|
|
||||||||
| /// Returns a `Key` pointer used internally by the storage API. | ||||||||
| /// | ||||||||
| /// This key is a combination of the `Mapping`'s internal `offset_key` | ||||||||
| /// and the user provided `key`. | ||||||||
| fn storage_key<Q>(&self, key: Q) -> Key | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have you benchmarked that this yields an improvement?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't need to specify the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just tried now, and there are no changes to the Wasm size. However, there are also no changes if I remove the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if the compiler also takes into account that passing large types (> 64bit) is very expensive in wasm. Ideally, |
||||||||
| where | ||||||||
| Q: scale::EncodeLike<K>, | ||||||||
| { | ||||||||
| let encodedable_key = (&self.offset_key, key); | ||||||||
| let mut output = <Blake2x256 as HashOutput>::Type::default(); | ||||||||
| ink_env::hash_encoded::<Blake2x256, _>(&encodedable_key, &mut output); | ||||||||
| output.into() | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| impl<K, V> SpreadLayout for Mapping<K, V> { | ||||||||
| const FOOTPRINT: u64 = 1; | ||||||||
| const REQUIRES_DEEP_CLEAN_UP: bool = false; | ||||||||
|
|
||||||||
| #[inline] | ||||||||
| fn pull_spread(ptr: &mut KeyPtr) -> Self { | ||||||||
| // Note: There is no need to pull anything from the storage for the | ||||||||
| // mapping type since it initializes itself entirely by the | ||||||||
| // given key pointer. | ||||||||
| Self::new(*ExtKeyPtr::next_for::<Self>(ptr)) | ||||||||
| } | ||||||||
|
|
||||||||
| #[inline] | ||||||||
| fn push_spread(&self, ptr: &mut KeyPtr) { | ||||||||
| // Note: The mapping type does not store any state in its associated | ||||||||
| // storage region, therefore only the pointer has to be incremented. | ||||||||
| ptr.advance_by(Self::FOOTPRINT); | ||||||||
| } | ||||||||
|
|
||||||||
| #[inline] | ||||||||
| fn clear_spread(&self, ptr: &mut KeyPtr) { | ||||||||
| // Note: The mapping type is not aware of its elements, therefore | ||||||||
| // it is not possible to clean up after itself. | ||||||||
| ptr.advance_by(Self::FOOTPRINT); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| impl<K, V> SpreadAllocate for Mapping<K, V> { | ||||||||
| #[inline] | ||||||||
| fn allocate_spread(ptr: &mut KeyPtr) -> Self { | ||||||||
| // Note: The mapping type initializes itself entirely by the key pointer. | ||||||||
| Self::new(*ExtKeyPtr::next_for::<Self>(ptr)) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| #[cfg(feature = "std")] | ||||||||
| const _: () = { | ||||||||
| use crate::traits::StorageLayout; | ||||||||
| use ink_metadata::layout::{ | ||||||||
| CellLayout, | ||||||||
| Layout, | ||||||||
| LayoutKey, | ||||||||
| }; | ||||||||
|
|
||||||||
| impl<K, V> StorageLayout for Mapping<K, V> | ||||||||
| where | ||||||||
| K: scale_info::TypeInfo + 'static, | ||||||||
| V: scale_info::TypeInfo + 'static, | ||||||||
| { | ||||||||
| fn layout(key_ptr: &mut KeyPtr) -> Layout { | ||||||||
| Layout::Cell(CellLayout::new::<Self>(LayoutKey::from( | ||||||||
| key_ptr.advance_by(1), | ||||||||
| ))) | ||||||||
| } | ||||||||
| } | ||||||||
| }; | ||||||||
|
|
||||||||
| #[cfg(test)] | ||||||||
| mod tests { | ||||||||
| use super::*; | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn insert_and_get_work() { | ||||||||
| ink_env::test::run_test::<ink_env::DefaultEnvironment, _>(|_| { | ||||||||
| let mut mapping: Mapping<u8, _> = Mapping::new([0u8; 32].into()); | ||||||||
| mapping.insert(&1, &2); | ||||||||
| assert_eq!(mapping.get(&1), Some(2)); | ||||||||
|
|
||||||||
| Ok(()) | ||||||||
| }) | ||||||||
| .unwrap() | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn gets_default_if_no_key_set() { | ||||||||
| ink_env::test::run_test::<ink_env::DefaultEnvironment, _>(|_| { | ||||||||
| let mapping: Mapping<u8, u8> = Mapping::new([0u8; 32].into()); | ||||||||
| assert_eq!(mapping.get(&1), None); | ||||||||
|
|
||||||||
| Ok(()) | ||||||||
| }) | ||||||||
| .unwrap() | ||||||||
| } | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I think of
EncodeLikesort-of likeEncodeLike = Borrow + Encode?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EncodeLike is a purely marker trait coming from the parity-scale-codec crate. Please go to the crates docs to read more about its general use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the docs, and I think my question is still valid 😄
I think the semantics of a type being able to be SCALE encoded as another type (
EncodeLike) vs. being able to be borrowed as another type (Borrow) are slightly different, which is why I asked