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

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

Merged
merged 1 commit into from
Jan 9, 2023
Merged

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

merged 1 commit into from
Jan 9, 2023

Conversation

Ank4n
Copy link
Contributor

@Ank4n Ank4n commented Jan 6, 2023 •
edited
Loading

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 approved these changes Jan 6, 2023
View reviewed changes
Copy link

@rossbulat rossbulat left a comment

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
Copy link
Contributor Author

Ank4n commented Jan 8, 2023 •
edited
Loading

Does this satisfy a convention in substrate?

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

melekes
melekes approved these changes Jan 9, 2023
View reviewed changes
Copy link
Contributor

@melekes melekes left a comment

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
@Ank4n
add note on usage of twox hash (#13089)
b8408bb
@aurexav aurexav mentioned this pull request Feb 8, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Feb 14, 2023
Merged
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
@Ank4n @ltfschoen
add note on usage of twox hash (paritytech#13089)
e468140
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@Ank4n @ark0f
add note on usage of twox hash (paritytech#13089)
e22bbf9
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rossbulat rossbulat rossbulat approved these changes

@kianenigma kianenigma kianenigma approved these changes

@melekes melekes melekes approved these changes

@ruseinov ruseinov Awaiting requested review from ruseinov

@gpestana gpestana Awaiting requested review from gpestana

Assignees
No one assigned
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
(Nominated) Proof of Stake
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ank4n @melekes @kianenigma @rossbulat