Pallet AURA: remove pallet::getter macro and write the corresponding code #3350

k-gunjan · 2024-02-16T07:35:24Z

Removed the pallet::getter macro call from storage type definitions and added the corresponding implementations directly.
fixes #3330

polkadot address: 14JzTPPUd8x8phKi8qLxHgNTnTMg6DUukCLXoWprejkaHXPz

cla-bot-2021 · 2024-02-16T07:35:30Z

User @k-gunjan, please sign the CLA here.

kianenigma · 2024-02-16T09:20:17Z

substrate/frame/aura/src/lib.rs

+	///
+	/// This function will be deprecated in future releases. It is recommended not to use it in new
+	/// developments.
+	pub fn authorities() -> BoundedVec<T::AuthorityId, T::MaxAuthorities> {


where are these functions used? do they even need to exist?

Thank you for your review and the points raised. I appreciate the opportunity to clarify the usage and implications of the proposed changes.

I've noted that the this function is utilized not only within this pallet like here but also across other crates within the SDK like here. This widespread use underscores its current necessity and potential impact on existing implementations.

In my understanding, the primary aim of this PR is to streamline the codebase by removing unnecessary macro complexity, thereby improving readability and maintainability without altering the existing functionality or interfaces. Given this scope, maintaining these function calls as-is ensures compatibility and stability across the SDK during this transitional phase. Please suggest .

If recommended I can go ahead and replace the uses here with Authorities::<T>::get() in a future iteration.

Let me know if you recommend proceeding with these adjustments now, or if we should consider them as part of a subsequent phase of optimization.

If recommended I can go ahead and replace the uses here with Authorities::::get() in a future iteration.

In my opinion we should do this. No need for another function.

Thank you for your valuable feedback. I've implemented the suggested changes in the latest commit (e81e6c4). You can view the changes here.

Please take a look at the updated code when you have a moment. I'm open to any further suggestions or discussions if there's more to refine.

@k-gunjan I think Oliver meant that you should not introduce these new methods in this PR.
Instead, you should go around the other crates that call the getter functions you removed, and replace them with pallet_aura::Authorities::<T>::get().

Thanks Muraca ! I'll remove definition from here and replace uses in other pallets.

muraca · 2024-02-19T11:35:18Z

substrate/frame/aura/src/lib.rs

+	///
+	/// This function will be deprecated in future releases. It is recommended not to use it in new
+	/// developments.
+	pub fn authorities() -> BoundedVec<T::AuthorityId, T::MaxAuthorities> {


@k-gunjan I think Oliver meant that you should not introduce these new methods in this PR.
Instead, you should go around the other crates that call the getter functions you removed, and replace them with pallet_aura::Authorities::<T>::get().

s0me0ne-unkn0wn

LGTM! 👍

muraca · 2024-03-08T10:50:46Z

As a breaking change, this requires the removal of the R0 label and a prdoc

…ue-3330

substrate/frame/aura/src/lib.rs

…code (paritytech#3350) Removed the `pallet::getter` macro call from storage type definitions and added the corresponding implementations directly. fixes paritytech#3330 polkadot address: 14JzTPPUd8x8phKi8qLxHgNTnTMg6DUukCLXoWprejkaHXPz --------- Co-authored-by: Bastian Köcher <git@kchr.de>

k-gunjan · 2024-04-24T06:14:05Z

tip?

- Update weights to reflect the new version. Notable Changes: - [System Callbacks](paritytech/polkadot-sdk#1781) - [Remove experimental flag](https://github.com/paritytech/polkadot-sdk/pull/3654/files) - [Remove pallet::getter macr](paritytech/polkadot-sdk#3350) - [Refactor APIs](https://github.com/paritytech/polkadot-sdk/pull/3817/files#diff-b02373af4015a8ebdf3a3f5be9ea0ce555b6e45331872e0465fd2f488177d383) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.10.0) issue-1922

- Update weights to reflect the new version. Notable Changes: - [Remove experimental flag](https://github.com/paritytech/polkadot-sdk/pull/3654/files) - [Remove pallet::getter macr](paritytech/polkadot-sdk#3350) - [Refactor APIs](https://github.com/paritytech/polkadot-sdk/pull/3817/files#diff-b02373af4015a8ebdf3a3f5be9ea0ce555b6e45331872e0465fd2f488177d383) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.10.0) issue-1922

- Update weights to reflect the new version. Notable Changes: - [Remove experimental flag](https://github.com/paritytech/polkadot-sdk/pull/3654/files) - [Remove pallet::getter macr](paritytech/polkadot-sdk#3350) - [Refactor APIs](https://github.com/paritytech/polkadot-sdk/pull/3817/files#diff-b02373af4015a8ebdf3a3f5be9ea0ce555b6e45331872e0465fd2f488177d383) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.10.0) issue-1928

- Upgrade Polkadot-sdk to v.1.10.0. - Update weights to reflect the new version. Notable Changes: - [Remove experimental flag](https://github.com/paritytech/polkadot-sdk/pull/3654/files) - [Remove pallet::getter macr](paritytech/polkadot-sdk#3350) - [Refactor APIs](https://github.com/paritytech/polkadot-sdk/pull/3817/files#diff-b02373af4015a8ebdf3a3f5be9ea0ce555b6e45331872e0465fd2f488177d383) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.10.0) issue-1928

- Upgrade Polkadot-sdk 1.9.0 to 1.10.0 - Update weights to reflect the new version. Notable Changes: - [Remove experimental flag](https://github.com/paritytech/polkadot-sdk/pull/3654/files) - [Remove pallet::getter macro](paritytech/polkadot-sdk#3350) - [Refactor APIs](https://github.com/paritytech/polkadot-sdk/pull/3817/files#diff-b02373af4015a8ebdf3a3f5be9ea0ce555b6e45331872e0465fd2f488177d383) - [Refactor Unified Host Functions](paritytech/polkadot-sdk#3854) - [StorageWeightReclaim SignedExtension](https://github.com/paritytech/polkadot-sdk/pull/3002/files) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.10.0)

- Upgrade Polkadot-sdk 1.9.0 to 1.10.0 - Update weights to reflect the new version. Notable Changes: - [Remove experimental flag](https://github.com/paritytech/polkadot-sdk/pull/3654/files) - [Remove pallet::getter macro](paritytech/polkadot-sdk#3350) - [Refactor APIs](https://github.com/paritytech/polkadot-sdk/pull/3817/files#diff-b02373af4015a8ebdf3a3f5be9ea0ce555b6e45331872e0465fd2f488177d383) - [Refactor Unified Host Functions](paritytech/polkadot-sdk#3854) - [StorageWeightReclaim SignedExtension](https://github.com/paritytech/polkadot-sdk/pull/3002/files) For more details, please refer to: [Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.10.0) #1928 --------- Co-authored-by: enddynayn <enddynayn@users.noreply.github.com>

k-gunjan requested a review from a team as a code owner February 16, 2024 07:35

kianenigma reviewed Feb 16, 2024

View reviewed changes

davxy changed the title ~~remove pallet::getter macro and write the corresponding code~~ Pallet AURA: remove pallet::getter macro and write the corresponding code Feb 16, 2024

kianenigma approved these changes Feb 19, 2024

View reviewed changes

franciscoaguirre added the R0-silent Changes should not be mentioned in any release notes label Feb 19, 2024

muraca mentioned this pull request Feb 19, 2024

[Deprecation] remove pallet::getter usage from all pallets #3326

Open

63 tasks

muraca suggested changes Feb 19, 2024

View reviewed changes

s0me0ne-unkn0wn approved these changes Mar 5, 2024

View reviewed changes

k-gunjan force-pushed the gunjan-remove-getter-macro-pallet-aura-issue-3330 branch 3 times, most recently from 005d91f to 05ab1ae Compare March 6, 2024 04:12

k-gunjan added 18 commits March 6, 2024 15:44

remove pallet::getter macro and write the corresponding code

7b95a13

replaced call to fun authorities() with ::get() API

9d3c70d

removed authorities and current_slot calls from pallet_aura

3d08424

refactor node-template-runtime

b3da689

resolve conflict

bbc09f7

refactor parachain template

64d947e

refactor people-westend runtime

70f9ded

refactor people rococo runtime

6dbffeb

refactor bridge-hub-rococo-runtime

9e13097

refactor bridge-hub-westend-runtime

e42fee4

rococo-parachain-runtime

ec3602e

refactor penpal runtime

83e1693

glutton-westend-runtime

f217fff

refactor shell runtime

b87d23a

refactor seedling runtime

8cd5fab

collectives-westend-runtime

e517dce

refactor asset-hub-rococo-runtime

20be756

refactor asset-hub-westend-runtime

a9ded7b

refactor to match master branch

9b59665

k-gunjan force-pushed the gunjan-remove-getter-macro-pallet-aura-issue-3330 branch from 9c1a94c to 9b59665 Compare March 6, 2024 10:15

k-gunjan added 2 commits March 15, 2024 17:45

prdoc for paritytech#3350

930e4c7

Merge branch 'master' into gunjan-remove-getter-macro-pallet-aura-iss…

1eb4bea

…ue-3330

bkchr approved these changes Mar 18, 2024

View reviewed changes

substrate/frame/aura/src/lib.rs Outdated Show resolved Hide resolved

substrate/frame/aura/src/lib.rs Outdated Show resolved Hide resolved

substrate/frame/aura/src/lib.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

0008aca

bkchr enabled auto-merge March 18, 2024 10:04

bkchr added this pull request to the merge queue Mar 18, 2024

Merged via the queue into paritytech:master with commit 816c072 Mar 18, 2024
130 of 132 checks passed

k-gunjan deleted the gunjan-remove-getter-macro-pallet-aura-issue-3330 branch March 18, 2024 10:55

Morganamilo mentioned this pull request Apr 4, 2024

PRDoc crate semver #3984

Closed

12 tasks

enddynayn mentioned this pull request Jul 30, 2024

chore(upgrade): v1.9.0 to v1.10.0 frequency-chain/frequency#2104

Merged

chungquantin mentioned this pull request Aug 13, 2024

chore: upgrade to 1.10.0 r0gue-io/pop-node#182

Merged

TDemeco mentioned this pull request Sep 27, 2024

feat: ⏫ upgrade to Polkadot SDK v1.10.0 Moonsong-Labs/storage-hub#210

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pallet AURA: remove pallet::getter macro and write the corresponding code #3350

Pallet AURA: remove pallet::getter macro and write the corresponding code #3350

k-gunjan commented Feb 16, 2024 •

edited

Loading

cla-bot-2021 bot commented Feb 16, 2024

kianenigma Feb 16, 2024

k-gunjan Feb 16, 2024 •

edited

Loading

ggwpez Feb 16, 2024

k-gunjan Feb 16, 2024

muraca Feb 19, 2024

k-gunjan Feb 19, 2024

muraca Feb 19, 2024

s0me0ne-unkn0wn left a comment

muraca commented Mar 8, 2024

k-gunjan commented Apr 24, 2024

Pallet AURA: remove pallet::getter macro and write the corresponding code #3350

Pallet AURA: remove pallet::getter macro and write the corresponding code #3350

Conversation

k-gunjan commented Feb 16, 2024 • edited Loading

cla-bot-2021 bot commented Feb 16, 2024

kianenigma Feb 16, 2024

Choose a reason for hiding this comment

k-gunjan Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

ggwpez Feb 16, 2024

Choose a reason for hiding this comment

k-gunjan Feb 16, 2024

Choose a reason for hiding this comment

muraca Feb 19, 2024

Choose a reason for hiding this comment

k-gunjan Feb 19, 2024

Choose a reason for hiding this comment

muraca Feb 19, 2024

Choose a reason for hiding this comment

s0me0ne-unkn0wn left a comment

Choose a reason for hiding this comment

muraca commented Mar 8, 2024

k-gunjan commented Apr 24, 2024

k-gunjan commented Feb 16, 2024 •

edited

Loading

k-gunjan Feb 16, 2024 •

edited

Loading