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

Generate storage info for Pallet Staking #9864

Closed
wants to merge 82 commits into from
Closed

Generate storage info for Pallet Staking #9864

wants to merge 82 commits into from

Conversation

georgesdib
Copy link
Contributor

@georgesdib georgesdib commented Sep 25, 2021

Fixes part of paritytech/polkadot-sdk#323

Adding generate_storage_info to pallet Staking.

kusama address: EawudhGYQCeKvpjUiXxRdn2bmSWUxkA1dndMHoYFVmQvRmi

Polkadot companion: paritytech/polkadot#3936

skip check-dependent-cumulus
(Need that because cumulus breaks from polkadot changes)

Summary of changes below:

  • Documentation Changes:
  1. Fixed error in URL link.
  2. Update documentation of pallet example to FRAME 2.0
  • Converting Vec to BoundedVec and WeakBoundedVec -> Needed to implement storage_info.

  • Moving some constants to become runtime parameters:

  1. MAX_NOMINATIONS -> MaxNominations
  2. MAXIMUM_VOTES_PER_VOTER -> MaximumVotesPerVoter
  3. MAX_UNLOCKING_CHUNKS -> MaxUnlockingChunks as per @kianenigma suggestion
  • Renaming MaxNominatorRewardedPerValidator to MaxRewardableIndividualExposures as per @kianenigma suggestion (@thiolliere is that OK with you?).

  • Implementation some needed utility functions on BoundedVec and WeakBoundedVec.

  • Implementing derive OrdNoBound for structs only, to avoid implementing it manually on Exposure.

  • Using ConstU32 as opposed to declaring in parameter_types inline with �Replace parameter_types with ConstU32 &c. #10402

  • The most controversial change probably, making MaxValidatorsCount become a runtime parameter instead of a storage item. This means, in particular, that updating that value will need a runtime configuration change as opposed to a mere extrinsic call. It also means, we always need a bound and it cannot be unset to leave it unbounded.

Georges Dib added 27 commits September 20, 2021 22:11
to pallet staking.
Moved MAX_UNLOCKING_CHUNKS from being a constant to being a type
called `MaxUnlockingChunks`
`MaxNominations` to allow it to be a bound for a `BoundedVec`
storage to being a runtime parameter `MaxNbOfValidators`
Fixing various other compile errors
@georgesdib
Copy link
Contributor Author

@kianenigma does the PR look good to you? Also if possible, if you can please check the polkadot companion and add the needed tags as I don't have the permission to do so. I think that is the reason for the failed check.

@kianenigma
Copy link
Contributor

@georgesdib I personally really like this PR and have tried reviewing it in the past, but it is quite a big one and it touches some very sensitive code. Unfortunately, I can't promise when I will get the chance to do a full review, but I intend to do so.

One possibility is that after doing another review, I might ask to strip down this PR to something slightly simpler so that we can review and merge it in smaller chunks.

Thanks for your contribution so far. Hopefully we can push it to the finish line, and propose an onchain tip for your work.

@georgesdib
Copy link
Contributor Author

@georgesdib I personally really like this PR and have tried reviewing it in the past, but it is quite a big one and it touches some very sensitive code. Unfortunately, I can't promise when I will get the chance to do a full review, but I intend to do so.

One possibility is that after doing another review, I might ask to strip down this PR to something slightly simpler so that we can review and merge it in smaller chunks.

Thanks for your contribution so far. Hopefully we can push it to the finish line, and propose an onchain tip for your work.

Thank you!

@kianenigma
Copy link
Contributor

@georgesdib unfortunately as it stands now, the scope of this PR is too big and too sensitive (staking being one of the most critical components of any network) to be reviewed and merged in a timely manner. This PR has already been open for far too long and I rather close it for now so we can move forward, but propose the following follow-ups:

  1. Please put your Polkadot/Kusama address in the PR description, and I will make sure a tip is opened for your work in the corresponding network.
  2. I already started making pallet-staking bounded in smaller bike-size pieces (see the PR that linked to this recently), and I would like to invite you to DM me if you feel like helping further with that. I don't have a set of issues written down now, but I can probably come up with a list that you can work on.

Sorry for the inconvenience, and one last time, thanks for your contribution to Substrate!

@kianenigma kianenigma closed this Jan 11, 2022
@shawntabrizi
Copy link
Member

/tip medium

1 similar comment
@shawntabrizi
Copy link
Member

/tip medium

@substrate-tip-bot
Copy link

A medium tip was successfully submitted for georgesdib (EawudhGYQCeKvpjUiXxRdn2bmSWUxkA1dndMHoYFVmQvRmi on kusama).

https://polkadot.js.org/apps/#/treasury/tips

@shawntabrizi shawntabrizi removed the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited label Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants