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

[Bug]: GetParams errors when introducing new param #17826

Closed
1 task done
czarcas7ic opened this issue Sep 20, 2023 · 1 comment
Closed
1 task done

[Bug]: GetParams errors when introducing new param #17826

czarcas7ic opened this issue Sep 20, 2023 · 1 comment
Labels

Comments

@czarcas7ic
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

What happened:

When introducing a new parameter, the new parameter must be manually set, even if the desired value of that parameter is nil. See the following upgrade handler: https://github.com/osmosis-labs/osmosis/blob/54261649fd3d1e372749261430de642cebd9e95a/app/upgrades/v20/upgrades.go#L13-L32

RunMigrations is run which calls initGenesis. If I were to cal GetParams in the upgrade handler, we would get the following error (truncated for brevity):

"recovered: UnmarshalJSON cannot decode empty bytes\nstack:\ngoroutine 178
          [running]:\nruntime/debug.Stack()\n\truntime/debug/stack.go:24 +0x64\ngithub.com/cosmos/cosmos-sdk/baseapp.newDefaultRecoveryMiddleware.func1({0x231a320,
          0x400649f960})\n\tgithub.com/cosmos/[email protected]/baseapp/recovery.go:71 +0x24\ngithub.com/cosmos/cosmos-sdk/baseapp.newRecoveryMiddleware.func1({0x231a320?,
          0x400649f960?})\n\tgithub.com/cosmos/[email protected]/baseapp/recovery.go:39 +0x34\ngithub.com/cosmos/cosmos-sdk/baseapp.processRecovery({0x231a320,
          0x400649f960}, 0x400000e270?)\n\tgithub.com/cosmos/[email protected]/baseapp/recovery.go:28
          +0x38\ngithub.com/cosmos/cosmos-sdk/baseapp.processRecovery({0x231a320, 0x400649f960},
          0x2df5560?)\n\tgithub.com/cosmos/[email protected]/baseapp/recovery.go:33 +0x60\ngithub.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runTx.func1()\n\tgithub.com/cosmos/[email protected]/baseapp/baseapp.go:620
          +0xa8\npanic({0x231a320, 0x400649f960})\n\truntime/panic.go:884 +0x1f4\ngithub.com/cosmos/cosmos-sdk/x/params/types.Subspace.Get({{_,
          _}, _, {_, _}, {_, _}, {_, _, _}, ...}, ...)\n\tgithub.com/cosmos/[email protected]/x/params/types/subspace.go:109
          +0x20c\ngithub.com/cosmos/cosmos-sdk/x/params/types.Subspace.GetParamSet({{_, _},

You also can't use SetParams, since that would require you to call GetParams to get the current state which would have the same error above. To get around this, I exposed the Set command at each module so we can manually set the new value in the upgrade handler.

My recommendation would be that, when a new value gets added to the ParamKeyTable, the DefaultValue of that type is automatically assigned. This would allow for simpler manipulation at the upgrade handler level rather than hacking around it as I did.

Not really a massive issue, just a QoL that will likely save other chains time in the future.

Cosmos SDK Version

0.45 (osmosis fork)

How to reproduce?

No response

@tac0turtle
Copy link
Member

Hey the params modules has been marked as maintenance mode only. We have migrated away from it for many reasons. Unfortunately we will not be able to fix this issue. I know osmosis is migrating to 0.50, this will fix your issue

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

No branches or pull requests

2 participants