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

Register validation function for governance parameter set changes #3473

Closed
rigelrozanski opened this issue Feb 1, 2019 · 4 comments · Fixed by #5359
Closed

Register validation function for governance parameter set changes #3473

rigelrozanski opened this issue Feb 1, 2019 · 4 comments · Fixed by #5359
Assignees
Milestone

Comments

@rigelrozanski
Copy link
Contributor

CC @sunnya97

we need a way to register a function which validates whether a parameter changes is within the accepted constraints and reject parameter set change transactions which are outside of that range. To change the range of a parameter would mean a fork.

@rigelrozanski
Copy link
Contributor Author

maybe actually belongs as a part of the params store?

@fedekunze
Copy link
Collaborator

is this already implemented cc: @alexanderbez ? if so can we close it ?

@alexanderbez
Copy link
Contributor

alexanderbez commented May 28, 2019

Not really @fedekunze.

What the parameter change proposal process currently does is validates that the given parameter changes are valid (e.g. correct type, correct key, etc)...it does not verify them (i.e. changing MaxValidators from 100 to 1000).

Currently, the process documents that these should be carefully evaluated by the proposer and stake holders. We're counting on the governance process to ensure this at the time being. In other words, no one would deposit or vote on a proposal that changes a param to some extreme value. However, what @rigelrozanski is stating is that this verification should reside on chain.

To do this, the param store would need to be modified such that when you register a param and its type, you also have to provide a validation function. This shouldn't be terribly difficult but it's also not super pressing for the reasons I described above.

I can assign this to myself or guide someone through this.

@alexanderbez
Copy link
Contributor

Solution to this is outlined in: #4069

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants