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

feat: introduce pallet-parameters to Westend to parameterize inflation #4938

Conversation

marcuspang
Copy link
Contributor

@marcuspang marcuspang commented Jul 3, 2024

Fixes #4907

Changes

  • Add pallet-parameters to Westend runtime
  • Add relay_era_payout function to Polkadot runtime common + deprecate previous era_payout function
    • Add unit tests for relay_era_payout, matching previous unit tests for era_payout
  • Remove pallet_staking_reward_curve in Westend runtime
  • Add dynamic_params to replace constants used for pallet_staking_reward_curve

Polkadot address: 5DybbVdLvVWd7MiA1YwkzLUNL1ThMDicozbuLc8v9pQEScPQ

@cla-bot-2021
Copy link

cla-bot-2021 bot commented Jul 3, 2024

User @marcuspang, please sign the CLA here.

@marcuspang marcuspang marked this pull request as ready for review July 5, 2024 14:05
@marcuspang
Copy link
Contributor Author

@kianenigma may I ask how I should benchmark and get the weight info? I'm not sure if I should run it with the Github action to derive the values

@kianenigma
Copy link
Contributor

bot help

@command-bot
Copy link

command-bot bot commented Jul 8, 2024

Here's a link to docs

@kianenigma
Copy link
Contributor

bot bench polkadot-pallet --pallet=pallet_parameters --runtime=westend

@command-bot
Copy link

command-bot bot commented Jul 8, 2024

@kianenigma https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6652919 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=pallet_parameters. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-4d068480-b725-4d5d-aef6-ce16e250a21a to cancel this command or bot cancel to cancel all commands in this pull request.

@kianenigma
Copy link
Contributor

Let's see if this works :)

@command-bot
Copy link

command-bot bot commented Jul 8, 2024

@kianenigma Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=pallet_parameters has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6652919 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6652919/artifacts/download.

@kianenigma
Copy link
Contributor

@marcuspang can you please add your address for a tip to the pr description?

@marcuspang
Copy link
Contributor Author

@marcuspang can you please add your address for a tip to the pr description?

Done!

@kianenigma
Copy link
Contributor

/tip medium

Copy link

@marcuspang Contributor did not properly post their account address.

Make sure the pull request description (or user bio) has: "{network} address: {address}".

@kianenigma
Copy link
Contributor

/tip medium

1 similar comment
@kianenigma
Copy link
Contributor

/tip medium

Copy link

@kianenigma Invalid tip size. Please specify one of small, medium, large.

@kianenigma
Copy link
Contributor

/tip medium

Copy link

@kianenigma A referendum for a medium (80 DOT) tip was successfully submitted for @marcuspang (5DybbVdLvVWd7MiA1YwkzLUNL1ThMDicozbuLc8v9pQEScPQ on polkadot).

Referendum number: 1021.
tip

Copy link

The referendum has appeared on Polkassembly.


#[cfg(feature = "runtime-benchmarks")]
fn try_successful_origin(_key: &RuntimeParametersKey) -> Result<RuntimeOrigin, ()> {
// Provide the origin for the parameter returned by `Default`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Provide the origin for the parameter returned by `Default`:
// Provide the origin for the parameter returned by `Default`.

Comment on lines +93 to +94
/// A specialized function to compute the inflation of the staking system, tailored for polkadot
/// relay chains, such as Polkadot, Kusama and Westend.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// A specialized function to compute the inflation of the staking system, tailored for polkadot
/// relay chains, such as Polkadot, Kusama and Westend.
/// A specialized function to compute the inflation of the staking system.

Nit but in this runtime it is implicit that the configs/impls are for Westend relay chain.

@@ -371,6 +393,46 @@ mod tests {
t.into()
}

pub fn deprecated_era_payout(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this fn is not used anywhere but in the tests, does it need #[allow(dead_code)] to avoid compilation warnings?

I'd probably have moved this implementation and the tests into a mod test_inflation {} as we probably dont use this code in the runtime anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit but moving this code under a mod gated by cfg(test) also make sense given that in the prdoc, it says that the old era payout impl has been removed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it currently lives under

#[cfg(test)]
mod tests {

in L265

@kianenigma kianenigma added this pull request to the merge queue Jul 22, 2024
Merged via the queue into paritytech:master with commit 6e520eb Jul 22, 2024
161 of 163 checks passed
ordian added a commit that referenced this pull request Jul 23, 2024
* master:
  Bump slotmap from 1.0.6 to 1.0.7 (#5096)
  feat: introduce pallet-parameters to Westend to parameterize inflation (#4938)
  Bump openssl from 0.10.64 to 0.10.66 (#5107)
  Bump lycheeverse/lychee-action from 1.9.1 to 1.10.0 (#5094)
  docs: remove the duplicate word (#5095)
  Prepare PVFs if node is a validator in the next session (#4791)
  Update parity publish (#5105)
@kianenigma
Copy link
Contributor

@ggwpez @EgorPopelyaev what is the way to track "when this will land on westend"? is there a public chat/agenda about when westend gets updated?

TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
paritytech#4938)

Fixes paritytech#4907

## Changes
- Add `pallet-parameters` to Westend runtime
- Add `relay_era_payout` function to Polkadot runtime common + deprecate
previous `era_payout` function
- Add unit tests for `relay_era_payout`, matching previous unit tests
for `era_payout`
- Remove `pallet_staking_reward_curve` in Westend runtime
- Add `dynamic_params` to replace constants used for
`pallet_staking_reward_curve`

Polkadot address: 5DybbVdLvVWd7MiA1YwkzLUNL1ThMDicozbuLc8v9pQEScPQ

---------

Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: kianenigma <[email protected]>
Co-authored-by: command-bot <>
@ggwpez
Copy link
Member

ggwpez commented Aug 6, 2024

what is the way to track "when this will land on westend"? is there a public chat/agenda about when westend gets updated?

I think unfortunately there is no way to easily see this. Sometimes devops posts in chat... yea we need some "Release Comms".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Westend Inflation Parameterize-able
6 participants