Problem: x/params is deprecated, need to migrate away from it

[adu-crypto]

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
close #632

According to the recent Cosmos-SDK ADR 046: Module Params , we should migrate x/cronos module away from x/params to enable stateful params changes, as well as more efficient params serialization.

• store params in x/cronos store instead of subspace
• create and handle MsgUpdateParams for grpc msgServer
• support querying params of x/cronos
• write and register migration plan
• add migration test

PR Checklist:

• Have you read the CONTRIBUTING.md?
• Does your PR follow the C4 patch requirements?
• Have you rebased your work on top of the latest master?
• Have you checked your code compiles? (make)
• Have you included tests for any non-trivial functionality?
• Have you checked your code passes the unit tests? (make test)
• Have you checked your code formatting is correct? (go fmt)
• Have you checked your basic code style is fine? (golangci-lint run)
• If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
• If your changes affect the client infrastructure, have you run the integration test?
• If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
• If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
• If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

[thomas-nguy]

does it needs to be handled in other modules as well ? (ibc-go, gravity-bridge, ethermint)

[adu-crypto]

does it needs to be handled in other modules as well ? (ibc-go, gravity-bridge, ethermint)

I think so as these modules also use the legacy subspace to get/set params
but the migration plan that I'm writing is restricted to the only cronos module.
do you think we should migrate these modules together

[codecov]

Codecov Report

Merging #735 (8e11a2b) into main (be8d53d) will increase coverage by 0.26%.
The diff coverage is 47.72%.

@@            Coverage Diff             @@
##             main     #735      +/-   ##
==========================================
+ Coverage   33.99%   34.26%   +0.26%     
==========================================
  Files          28       30       +2     
  Lines        1506     1582      +76     
==========================================
+ Hits          512      542      +30     
- Misses        941      983      +42     
- Partials       53       57       +4     

Impacted Files	Coverage Δ
x/cronos/keeper/grpc_query.go	0.00% <0.00%> (ø)
x/cronos/types/keys.go	0.00% <ø> (ø)
x/cronos/types/messages.go	14.38% <0.00%> (-3.16%)	⬇️
x/cronos/keeper/migrations.go	40.00% <40.00%> (ø)
x/cronos/keeper/keeper.go	76.04% <60.00%> (-1.20%)	⬇️
x/cronos/migrations/v2/migrate.go	72.72% <72.72%> (ø)
x/cronos/module.go	53.16% <75.00%> (+1.10%)	⬆️
x/cronos/keeper/params.go	73.52% <80.00%> (+0.80%)	⬆️
x/cronos/genesis.go	64.28% <100.00%> (ø)
x/cronos/keeper/msg_server.go	16.92% <100.00%> (+13.35%)	⬆️
... and 2 more

[yihuang]

Is this PR ready, can you add an integration test for the gov param message?

[adu-crypto]

Is this PR ready, can you add an integration test for the gov param message?

yeah sure. I will add the integration test soon

[yihuang]

Is this PR ready, can you add an integration test for the gov param message?

yeah sure. I will add the integration test soon

and ideally check the migration is succesfully in the upgrade integration test.

[adu-crypto]

@yihuang all checks has passed except protobuf breakage, how could I escape this check?

[yihuang]

LGTM overall, can you do a gov proposal to update parameters in integration test?

[adu-crypto]

LGTM overall, can you do a gov proposal to update parameters in integration test?

sure, add soon

[yihuang]

can you fix the conflicts?

[adu-crypto]

can you fix the conflicts?

No problem but test_gov_update_params failed for

AssertionError: (cronosd tx gov submit-proposal /private/tmp/pytest-of-adudu/pytest-2/test_gov_update_params0/proposal.json -y --home /private/tmp/pytest-of-adudu/pytest-2/cronos-gov0/cronos_777-1/node0 --gas-prices 100000000000basetcro --gas auto --gas-adjustment 1.5 --from community)

I'm still trying to fix this

[adu-crypto]

/runsim

[yihuang]

LGTM