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

ChangeParam tx doesn't reject an invalid value #1617

Open
msmania opened this issue Oct 2, 2024 · 1 comment
Open

ChangeParam tx doesn't reject an invalid value #1617

msmania opened this issue Oct 2, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@msmania
Copy link
Contributor

msmania commented Oct 2, 2024

If a new param is an invalid JSON string, or the type doesn't match the define one, the protocol just accepts without changing the state. The bug is this line doesn't respect the returned error.

_ = space.Update(ctx, []byte(paramKey), paramValue)

Ideally we should fix this on protocol side. Given that this product is maintenance mode, however, my suggestion is to add some check on client side to prevent it from submitting a tx with an invalid param value.

@msmania msmania added the bug Something isn't working label Oct 2, 2024
@Olshansk
Copy link
Member

Olshansk commented Oct 2, 2024

To provide some additional context, this is a consensus-breaking change that needs to be backwards compatible. To do so:

  1. Add a new governance parameter
  2. Add all the boilerplate around enabling / disabling it at the right height
  3. Upgrade the network
  4. Enable this field

Olshansk added a commit that referenced this issue Oct 2, 2024
TODO(#1617): Check the return value of `Update` and handle errors
appropriately
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

2 participants