-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
CLI should show outcome of parameter change proposals #4339
Comments
I don't follow @mossid. The proposal is already simulated when the msg is executed. In addition, the parameter(s) to be set are the ones exactly set in the proposal. What does this accomplish for us? Also, I imagine most, if not all, param change proposals will go through some form of social discourse (eg. forum post) initially where the exact proposal will be crafted in draft form and agreed upon by some majority before the proposal is actually made, at which we know exactly what is being changed. |
One specific example of the possible cases is excluding zero values on value. The golang json marshaller will omit omitempty fields if the values are set to zero value. So if the proposer does not use a handwritten json value, but instead a parameter value marshaled from a golang struct, which contains a zero value as its field, the marshaler will omit that and the parameter will not be changed on that field. So it is not for the failure of the logic itself(as it is tested on the msg execution as you said), but for the human mistake that can happen if there is no visible outcome displayed. I'm also pretty sure that the social discussion can catch this sort of human errors, but having another safety layer will be not bad. |
I don't quite follow the proposal or if it's worth the technical overhead, but if you believe the implementation to be minimal and trivial to implement, I'd be happy to review a PR 👍 |
I think this is the part I missed detailed explaination; it won't be, after #4247 is implemented. The proposal will only contain the fields that will be updated, not the whole parameter, so the parameters to be set are different from the ones set in the proposal. |
Correct, after #4247 is complete, the param change proposal only includes what's being changed so it should be self-evident. Am I missing something? |
are there any actionable items to do here then? |
I don't think there is anything to do here. Are you opposed to this being closed @mossid? |
Nope, okay to be closed 👍 @fedekunze @alexanderbez |
After #4247 is implemented, the parameter change proposal content will not completely override the existing parameter, potentially set different value from the proposers/voters' intention.
The CLI should show the expected parameter that will be set after the proposal passes, depending on the current parameter set, by locally simulating the parameter change proposal handler.
Separate the parameter overwriting logic from the parameter change proposal handler, and make it public
Make the CLI call this function with
(current_parameter, provided_update)
before submitting the transaction, and prints the expected updated parameterThe text was updated successfully, but these errors were encountered: