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

Move Schedule from storage to the Config trait #8433

Closed
4 of 5 tasks
athei opened this issue Mar 23, 2021 · 1 comment · Fixed by #8924
Closed
4 of 5 tasks

Move Schedule from storage to the Config trait #8433

athei opened this issue Mar 23, 2021 · 1 comment · Fixed by #8924
Labels
I7-refactor Code needs refactoring. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.

Comments

@athei
Copy link
Member

athei commented Mar 23, 2021

The Schedule is a struct that sees a lot of changes to its fields. The main reason is that every new contract callable function needs a new field. For this reason we made its fields private in #8359. Otherwise we would be burning through major version numbers every time we add a new function. This is not enough, though. Because the Schedule is in storage and also a parameter to the update_schedule dispatchable it is still exposed as a public API.

In order to solve this issue we remove the Schedule from storage and instead make it configurable through the Config type. This will drop the functionality to update the Schedule without a runtime upgrade, though. I argue that this feature wouldn't be that useful because the weight prices are already automatically adjusted depending on chain utilization. Larger changes would need some code changes in most cases. Any objections @pepyakin ?

  • Remove the CurrentSchedule item from the pallet.
  • Add storage migration which removes it from storage.
  • Add the Schedule: Get<Schedule> as an associated type to Config
  • Enable debug_println when called via RPC instead of relying on a genesis config as a toggle.
  • Make Schedule fields public in order to allow customization.

Making the fields private does not help us with avoiding major version bumps because the public WeightInfo trait still needs to change. We just accept the fact that we will have new major version numbers and expect runtimes to bump the version number and re-run benchmarks. Most of the time that will be everything that is to it.

Partly fixed by #8359 and #8773

@athei athei added I7-refactor Code needs refactoring. U1-asap No need to stop dead in your tracks, however issue should be addressed as soon as possible. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Mar 23, 2021
@pepyakin
Copy link
Contributor

That makes sense.

@athei athei removed the U1-asap No need to stop dead in your tracks, however issue should be addressed as soon as possible. label Mar 25, 2021
@ghost ghost closed this as completed in #8924 May 29, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants