v0.7 Upgrade with Storage and Readme Updates (for Cosmwasm v1.0) #66
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…, dequestore and keyset next
toml01
reviewed
Oct 27, 2022
…olkit into cw-v1.0-storage syncing my local copy
toml01
reviewed
Oct 31, 2022
packages/storage/src/deque_store.rs
Outdated
| let indexes = self._get_indexes(storage, indexes_page)?; | ||
| let item_data = indexes | ||
| .get(&index_pos) | ||
| .ok_or_else(|| StdError::generic_err("Item not found at this index."))?; |
There was a problem hiding this comment.
Shouldn't the error be "DequeStore out of bounds"?
There was a problem hiding this comment.
Not sure, I think we should only return out of bounds right after checking pos > len
packages/storage/src/deque_store.rs
Outdated
| if let Some(item_data) = indexes.get(&index_pos) { | ||
| item = Ser::deserialize(item_data); | ||
| } else { | ||
| item = Err(StdError::generic_err("Item not found at this index.")); |
There was a problem hiding this comment.
Also here, I think "out of bounds" is a more appropriate message
There was a problem hiding this comment.
Again, I only want to return out of bounds error right after checking pos >= len
toml01
reviewed
Nov 9, 2022
…ret-toolkit into scrtlabs-update-dep-versions
srdtrk
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reopening the PR after it was automatically closed when the old branch was deleted. It seems like this needs manual intervention to merge now (probably due to versioning changes)
The following changes were made to the storage as mentioned in release notes, these changes were approved by @reuvenpo conceptually:
Keysetstorage object (A hashset like storage object).KeymapBuilderandKeysetBuilderwhich allow the user to disable the iterator feature (saving gas) or adjust the internal indexes' page size so that the user may determine how many objects are to be stored/loaded together in the iterator.::new_with_page_size(namespace, page_size)method was added toAppendStoreandDequeStoreso that the user may adjust the internal indexes' page size which determine how many objects are to be stored/loaded together in the iterator.Keymap,AppendStore, andDequeStore.Moreover, I also fixed all the clippy warnings and updated some readme files to reflect cosmwasm 1.0 changes. Cosmwasm v1.0 branch should probably be made default