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

Require MaxEncodedLen per default #10652

Closed
ggwpez opened this issue Jan 13, 2022 · 1 comment · Fixed by #10662
Closed

Require MaxEncodedLen per default #10652

ggwpez opened this issue Jan 13, 2022 · 1 comment · Fixed by #10662
Labels
I7-refactor Code needs refactoring. U3-nice_to_have Issue is worth doing eventually. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.

Comments

@ggwpez
Copy link
Member

ggwpez commented Jan 13, 2022

Many pallets are currently missing MaxEncodedLen implementations. All future pallets should have it.
To make it easier not to miss it, we could either:

  • Add an optional bool argument to generate_storage_info which default to true
  • Remove generate_storage_info and add a without_storage_info attribute

What do you favor?

I personally like the second approach more, since we could then in the future remove both after all pallets have been migrated.

Related paritytech/polkadot-sdk#323

@ggwpez ggwpez added I7-refactor Code needs refactoring. U3-nice_to_have Issue is worth doing eventually. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Jan 13, 2022
@shawntabrizi
Copy link
Member

I strongly prefer the second option. By default, we should make sure all pallets follow this storage definition requirement.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring. U3-nice_to_have Issue is worth doing eventually. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants