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

Adds ability to use default hasher in dev_mode for explicit key binding #14164

Merged
merged 11 commits into from
May 19, 2023
Merged

Adds ability to use default hasher in dev_mode for explicit key binding #14164

merged 11 commits into from
May 19, 2023

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented May 17, 2023 •
edited
Loading

Fixes #14118

Recently, we added the ability to specify hashers as _ in dev_mode, which are replaced by Blake2_128Concat. This PR allows using a default hasher in dev_mode in case of explicit key binding.

With these changes, a syntax like the below works in dev_mode but throws an error otherwise.

#[pallet::storage]
pub type Baz<T: Config> = StorageMap<Key = T::AccountId, Value = T::Balance>;

@gupnik gupnik 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels May 17, 2023
@gupnik gupnik requested a review from a team May 17, 2023 04:08
@paritytech-ci paritytech-ci requested a review from a team May 17, 2023 07:13
sam0x17
sam0x17 approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@sam0x17 sam0x17 left a comment

Choose a reason for hiding this comment

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

looks good to me once those two are changed to parse_quote!

frame/support/procedural/src/pallet/parse/storage.rs Outdated Show resolved Hide resolved
ggwpez
ggwpez approved these changes May 17, 2023
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Looks good overall 👍

frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/parse/storage.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/parse/storage.rs Outdated Show resolved Hide resolved
@ggwpez ggwpez added B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. and removed B0-silent Changes should not be mentioned in any release notes labels May 17, 2023
@ggwpez
Copy link
Member

ggwpez commented May 17, 2023

And please add some description to the MR.
Otherwise the downstream teams will not know what is going on.

@gupnik
Copy link
Contributor Author

gupnik commented May 19, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented May 19, 2023 •
edited
Loading

@gupnik https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2859409 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 2-9d185688-6bbe-49ea-874d-736e795f9e39 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented May 19, 2023

@gupnik Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2859409 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2859409/artifacts/download.

@ggwpez ggwpez requested a review from KiChjang May 19, 2023 09:01
@gupnik
Copy link
Contributor Author

gupnik commented May 19, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 7c06185 into master May 19, 2023
@paritytech-processbot paritytech-processbot bot deleted the gupnik/dev-mode-explict-binding branch May 19, 2023 13:24
gpestana pushed a commit that referenced this pull request May 27, 2023
@gupnik @gpestana
Adds ability to use default hasher in dev_mode for explicit key bin…
9dcdd84 
…ding (#14164)

* Initial setup

* Minor update

* Minor update

* Addresses review comments

* Addresses review comments

* Updates doc

* ".git/.scripts/commands/fmt/fmt.sh"

* Renames file

* Updates path in test

---------

Co-authored-by: command-bot <>
@github-actions github-actions bot mentioned this pull request Jun 15, 2023
Closed
@kiltbot kiltbot mentioned this pull request Jun 21, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@gupnik @nathanwhit
Adds ability to use default hasher in dev_mode for explicit key bin…
0876fd0 
…ding (paritytech#14164)

* Initial setup

* Minor update

* Minor update

* Addresses review comments

* Addresses review comments

* Updates doc

* ".git/.scripts/commands/fmt/fmt.sh"

* Renames file

* Updates path in test

---------

Co-authored-by: command-bot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sam0x17 sam0x17 sam0x17 approved these changes

@KiChjang KiChjang KiChjang approved these changes

@ggwpez ggwpez ggwpez approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Explicit Key Binding should support default hasher in dev_mode
4 participants
@gupnik @ggwpez @sam0x17 @KiChjang