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

Why you use Addr as keys in Maps? #295

Closed
pronvis opened this issue Jun 2, 2021 · 4 comments · Fixed by #413
Closed

Why you use Addr as keys in Maps? #295

pronvis opened this issue Jun 2, 2021 · 4 comments · Fixed by #413
Assignees
Labels
documentation Improvements or additions to documentation storage plus
Milestone

Comments

@pronvis
Copy link
Contributor

pronvis commented Jun 2, 2021

Hey guys, thanks for your work!
Have a little question :)
Why you use Addr(which is a String actually) as a key in Maps? Here for example?

Here CosmWasm docs saying (and I agree):

This means that if message.signer is always the string address that signed
 the transaction and I use it to look up your account balance, if this encoding
 ever changed, then you lose access to your account. We clearly need
 a stable identifier to work with internally.

It is possible for address string representation to change, so ideally we all (smart contracts dev) should use CanonicalAddr as a key for Maps.

So, Why?

@ethanfrey
Copy link
Member

Until Stargate, the Cosmos SDK stored addresses internally as raw bytes (CanonicalAddr) form.
We reflected and considered this would make sense if they changed bech32 prefix or decided to move to base58 or whatever.

However, since 0.40 the cosmos sdk uses bech32 encoding string addresses in all messages and in storage. Thus, it is near impossible to change the address encoding on a Cosmos SDK chain. Since that limits the runtime, we don't have to make all our contracts significantly more complex to handle cases the runtime doesn't support.

We also suggest using Addr in storage for 0.14 plus. This also means doing addr_validate on any address passed in via a message to ensure it is a legitimite address and not "random-text-here" which will fail later. Thus pub fn addr_validate(&self, &str) -> Addr on deps.api.

@ethanfrey
Copy link
Member

We should update the cosmwasm docs for these new changes... Lots to do for 0.14 and 1.0 still...

@pronvis
Copy link
Contributor Author

pronvis commented Jun 4, 2021

Thanks much!
Should I close this issue?

@ethanfrey
Copy link
Member

No, leave it open. We need to improve the docs, then I will close it.

@ethanfrey ethanfrey added the documentation Improvements or additions to documentation label Jul 14, 2021
@ethanfrey ethanfrey added this to the v0.9.0 milestone Aug 17, 2021
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

Successfully merging a pull request may close this issue.

3 participants