Skip to content
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

Wrong/unclear explanation in IndexedMap docs #718

Closed
albertchon opened this issue May 12, 2022 · 1 comment
Closed

Wrong/unclear explanation in IndexedMap docs #718

albertchon opened this issue May 12, 2022 · 1 comment
Labels
documentation Improvements or additions to documentation storage plus

Comments

@albertchon
Copy link

The docs for IndexedMap state

Given that `token_id` is a string value, we specify `String` as the last argument of the `MultiIndex` definition.
That way, the deserialization of the primary key will be done to the right type (an owned string).

However in the latest cw-nfts code (using cw-storage-plus v0.13.2), this is no longer the case
https://github.com/CosmWasm/cw-nfts/blob/584e01526048334b87a85a6c69f54109727efb5a/contracts/cw721-base/src/state.rs#L121-L126

It seems this change arose from @maurolacy's removing of the PK default from MultiIndex, so that "the PK type must match the encompassing IndexedMap primary key type, or its owned variant".

@ethanfrey ethanfrey added documentation Improvements or additions to documentation storage plus labels Jun 17, 2022
@maurolacy
Copy link
Contributor

maurolacy commented Aug 26, 2022

cw-nfts code should be modified to conform to this new format / convention, yes.

Please note this is a convention, and no compilation error will be thrown if you don't follow it. You will most likely hit deserialisation errors, and / or range / iteration misses, particularly if you indicate deserialising the pk to an incompatible type through this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation storage plus
Projects
None yet
Development

No branches or pull requests

3 participants