Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

[trivial] add note on usage of twox hash#13089

Merged
Ank4n merged 1 commit into
masterfrom
ankan/twox-note
Jan 9, 2023
Merged

[trivial] add note on usage of twox hash#13089
Ank4n merged 1 commit into
masterfrom
ankan/twox-note

Conversation

@Ank4n

@Ank4n Ank4n commented Jan 6, 2023

Copy link
Copy Markdown
Contributor

Adding some note to justify using twox hasher for storage maps where key is user generated.

Longer explanation
By convention, a storage map that has user generated key should use a cryptographic hasher (such as blake2). For storage maps which are populated by a signed AccountId as key though, it is fine to use twox hasher since generated AccountId is itself a secure hash.

@Ank4n Ank4n added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Jan 6, 2023
@Ank4n Ank4n requested a review from kianenigma as a code owner January 6, 2023 14:09

@rossbulat rossbulat left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this satisfy a convention in substrate?

@Ank4n

Ank4n commented Jan 8, 2023

Copy link
Copy Markdown
Contributor Author

Does this satisfy a convention in substrate?

Added some more context in the description. Shawn's blog explains this in depth.

@melekes melekes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Ank4n Ank4n merged commit 911f65b into master Jan 9, 2023
@Ank4n Ank4n deleted the ankan/twox-note branch January 9, 2023 07:33
rossbulat pushed a commit that referenced this pull request Jan 11, 2023
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants