Remove usage of the pallet::getter macro from pallet-grandpa #4529
Conversation
substrate/frame/grandpa/src/lib.rs
Outdated
| @@ -430,6 +424,54 @@ pub enum StoredState<N> { | |||
| } | |||
|
|
|||
| impl<T: Config> Pallet<T> { | |||
| /// Get the tate of the current authority set | |||
| pub fn state() -> StoredState<BlockNumberFor<T>> { | |||
There was a problem hiding this comment.
Why did you started to add them here now?
There was a problem hiding this comment.
Indeed. Furthermore, if you absolutely need these accessors, each of them can be simplified. E.g.
pub fn state() -> StoredState<BlockNumberFor<T>> {
State::<T>::get()
}
There was a problem hiding this comment.
Ah good call on the simplification, I had just replaced them with what the macro generated. Will fix 👍
And I've added the explicit getters because the getter macro made the storage values read only outside the crate (since they're pub(super)), so taking the macro away reduced their visibility to other pallets. Usually this isn't an issue because most the storage values are pub, but I opted not to switch to pub because I feel preserving data authority is important, especially since I don't know the reasoning behind why they were made read-only originally.
There was a problem hiding this comment.
There is no real reasoning and anyone could write them by just writing to the storage items. So, just make the storage items public.
There was a problem hiding this comment.
Sounds good. I'll fix and subsequently do this moving forward.
|
The CI pipeline was cancelled due to failure one of the required jobs. |
As per #3326, removes pallet::getter macro usage from pallet-grandpa. The syntax
StorageItem::<T, I>::get()should be used instead.cc @muraca