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.

Sign up for GitHub

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

Jump to bottom

Remove pallet-getter usage from pallet-session #4972

Merged
merged 5 commits into from
Jul 16, 2024
Merged

Remove pallet-getter usage from pallet-session #4972

merged 5 commits into from
Jul 16, 2024
+83 −53

Conversation

mittal-parth
Copy link
Contributor

As per #3326, removes usage of the pallet::getter macro from the session pallet. The syntax StorageItem::<T, I>::get() should be used instead.

Also, adds public functions for compatibility.

NOTE: The ./historical directory has not been modified.

cc @muraca

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

@mittal-parth mittal-parth requested a review from a team as a code owner July 8, 2024 12:41
@bkchr bkchr added the T2-pallets This PR/Issue is related to a particular pallet. label Jul 11, 2024
@bkchr bkchr requested a review from ggwpez July 11, 2024 22:21
kianenigma
kianenigma reviewed Jul 15, 2024
View reviewed changes
prdoc/pr_4972.prdoc Outdated Show resolved Hide resolved
kianenigma
kianenigma reviewed Jul 15, 2024
View reviewed changes
substrate/frame/session/src/lib.rs
@@ -607,33 +603,53 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
/// Public function to access the current set of validators.
pub fn validators() -> Vec<T::ValidatorId> {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can already mark these as deprecated, and our codebase will be fine right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean not using the validators function?

It is being used in a lot of places outside of the session pallet like this

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say that deprecating the compatibility functions and changing them within the sdk is in scope for this PR. I had a quick flick through a couple of PRs from the linked issue and others have replaced usage of getters outside the pallet in similar situations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a little dependent. Even I had two ways on this. Some PRs have changed, some haven't. If this sounds, good I can replace all instances (outside the pallet as well).
cc: @kianenigma

@mittal-parth @kianenigma
Add empty space at end of prdoc
c4ef1d8 
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@github-actions GitHub Actions
Copy link

Review required! Latest push from author must always be reviewed

@bkchr bkchr enabled auto-merge July 16, 2024 08:21
@bkchr bkchr added this pull request to the merge queue Jul 16, 2024
Merged via the queue into paritytech:master with commit 0b3d760 Jul 16, 2024
153 of 160 checks passed
ordian added a commit that referenced this pull request Jul 17, 2024
@ordian
Merge branch 'master' into ao-paraheadrootsprovider-coretime-fix
0f207dd 
* master:
  add elastic scaling MVP guide (#4663)
  Send PeerViewChange with high priority (#4755)
  [ci] Update forklift in CI image (#5032)
  Adjust base value for statement-distribution regression tests (#5028)
  [pallet_contracts] Add support for transient storage in contracts host functions (#4566)
  [1 / 5] Optimize logic for gossiping assignments (#4848)
  Remove `pallet-getter` usage from pallet-session (#4972)
  command-action: added scoped permissions to the github tokens (#5016)
  net/litep2p: Propagate ValuePut events to the network backend (#5018)
  rpc: add back rpc logger (#4952)
  Updated substrate-relay version for tests (#5017)
  Remove most all usage of `sp-std` (#5010)
  Use sp_runtime::traits::BadOrigin (#5011)
jpserrat pushed a commit to jpserrat/polkadot-sdk that referenced this pull request Jul 18, 2024
@mittal-parth @kianenigma
@bkchr @jpserrat
Remove pallet-getter usage from pallet-session (paritytech#4972)
e8af1d7 
As per paritytech#3326, removes usage of the `pallet::getter` macro from the
`session` pallet. The syntax `StorageItem::<T, I>::get()` should be used
instead.

Also, adds public functions for compatibility.

NOTE: The `./historical` directory has not been modified.

cc @muraca

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
ordian added a commit that referenced this pull request Jul 18, 2024
@ordian
Merge branch 'master' into ao-fix-backing-filtering-on-disputes
f75dcd7 
* master: (125 commits)
  add elastic scaling MVP guide (#4663)
  Send PeerViewChange with high priority (#4755)
  [ci] Update forklift in CI image (#5032)
  Adjust base value for statement-distribution regression tests (#5028)
  [pallet_contracts] Add support for transient storage in contracts host functions (#4566)
  [1 / 5] Optimize logic for gossiping assignments (#4848)
  Remove `pallet-getter` usage from pallet-session (#4972)
  command-action: added scoped permissions to the github tokens (#5016)
  net/litep2p: Propagate ValuePut events to the network backend (#5018)
  rpc: add back rpc logger (#4952)
  Updated substrate-relay version for tests (#5017)
  Remove most all usage of `sp-std` (#5010)
  Use sp_runtime::traits::BadOrigin (#5011)
  network/tx: Ban peers with tx that fail to decode (#5002)
  Try State Hook for Bounties (#4563)
  [statement-distribution] Add metrics for distributed statements in V2 (#4554)
  added sync command (#4818)
  Bridges V2 refactoring backport and `pallet_bridge_messages` simplifications (#4935)
  xcm-executor: Improve logging (#4996)
  Remove usage of `sp-std` on templates (#5001)
  ...
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
@mittal-parth @kianenigma
@bkchr @TarekkMA
Remove pallet-getter usage from pallet-session (paritytech#4972)
c876a6f 
As per paritytech#3326, removes usage of the `pallet::getter` macro from the
`session` pallet. The syntax `StorageItem::<T, I>::get()` should be used
instead.

Also, adds public functions for compatibility.

NOTE: The `./historical` directory has not been modified.

cc @muraca

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
ordian added a commit that referenced this pull request Aug 6, 2024
@ordian
Merge branch 'master' into ao-account-for-enact-candidate-weight
e6978fd 
* master: (130 commits)
  add elastic scaling MVP guide (#4663)
  Send PeerViewChange with high priority (#4755)
  [ci] Update forklift in CI image (#5032)
  Adjust base value for statement-distribution regression tests (#5028)
  [pallet_contracts] Add support for transient storage in contracts host functions (#4566)
  [1 / 5] Optimize logic for gossiping assignments (#4848)
  Remove `pallet-getter` usage from pallet-session (#4972)
  command-action: added scoped permissions to the github tokens (#5016)
  net/litep2p: Propagate ValuePut events to the network backend (#5018)
  rpc: add back rpc logger (#4952)
  Updated substrate-relay version for tests (#5017)
  Remove most all usage of `sp-std` (#5010)
  Use sp_runtime::traits::BadOrigin (#5011)
  network/tx: Ban peers with tx that fail to decode (#5002)
  Try State Hook for Bounties (#4563)
  [statement-distribution] Add metrics for distributed statements in V2 (#4554)
  added sync command (#4818)
  Bridges V2 refactoring backport and `pallet_bridge_messages` simplifications (#4935)
  xcm-executor: Improve logging (#4996)
  Remove usage of `sp-std` on templates (#5001)
  ...
This was referenced Aug 21, 2024
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seadanda seadanda seadanda left review comments

@bkchr bkchr bkchr approved these changes

@ggwpez ggwpez Awaiting requested review from ggwpez

@kianenigma kianenigma Awaiting requested review from kianenigma

Assignees
No one assigned
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mittal-parth @kianenigma @bkchr @seadanda