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

Fixing ManualKey<0> to act properly #1670

Merged
merged 4 commits into from
Feb 23, 2023

Conversation

xgreenx
Copy link
Collaborator

@xgreenx xgreenx commented Feb 21, 2023

Previously the ManualKey<0> acted as an AutoKey without the possibility of really using the zero storage key. This fix allows using the zero manual key that overrides the auto-generated key.

…bility of really using the zero storage key. This fix allows using the zero manual key that overrides the auto-generated key.
<ResolverKey<ManualKey<456>, ManualKey<123>> as StorageKey>::KEY,
456
);
assert_eq!(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without the fix, it would fail with the 123 storage key=)

@cmichi
Copy link
Collaborator

cmichi commented Feb 22, 2023

Thanks for the PR, @xgreenx!

Can you add a changelog entry to the Unreleased section? I guess a patch release would be best, even though it's strictly speaking a breaking change.

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Thanks!

crates/storage/traits/src/impls/mod.rs Outdated Show resolved Hide resolved
crates/storage/traits/src/impls/mod.rs Outdated Show resolved Hide resolved
@HCastano HCastano merged commit e9321aa into use-ink:master Feb 23, 2023
@SkymanOne SkymanOne mentioned this pull request Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants