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

pallet-example-basic: Missing Hooks #6701

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rainbow-promise
Copy link

Resolves #6398

Proposed Implementation

  • pre_upgrade: Retrieve and assert the on-chain storage version, returning an empty Vec.
  • on_runtime_upgrade: Clear all storage items and raise the on-chain StorageVersion to 1.
  • post_upgrade: Assert the new on-chain storage version.

Notes

The try-state hook validates invariants in the storage items, as the pallet contains trivial logic this will be left unimplemented()

@cla-bot-2021
Copy link

cla-bot-2021 bot commented Nov 28, 2024

User @rainbow-promise, please sign the CLA here.

@rainbow-promise
Copy link
Author

Hello @ggwpez

A few questions:

  • Can we delete the unused Storage Item(Foo & Bar)?
  • Migrate Dummy to use StorageMap of [key: T::Account, value: T::Balance]?

#[pallet::storage]
pub(super) type Dummy<T: Config> = StorageValue<_, T::Balance>;
Copy link
Member

Choose a reason for hiding this comment

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

Instead of deleting them, please do something interesting with it as an example.

As this is an example pallet: it should showcase some sensible things that can be done with storage items etc. And it should document these things so that a beginner can understand it.

}

fn on_poll(_n: BlockNumberFor<T>, _weight: &mut WeightMeter) {
unimplemented!()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it isn't so good to have unimplemented here and also other places.
If user wants to experiment and modify the function and execute some tests then it will trigger everywhere.

If think it is better to just have an inner comment //

}
}
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

why removed? it is a good example.

@@ -169,7 +173,7 @@ fn counted_map_works() {
assert_eq!(CountedMap::<Test>::count(), 1);
})
}

*/
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe remove it completely or keep it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pallet-examples: Add missing hooks
3 participants