From 1db8f034f26f3d33233b86ca15ff94259b7695f0 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 15 Oct 2024 13:43:58 +0800 Subject: [PATCH] Problem: expedited related gov params are not adjusted (#1087) * Problem: expedited related gov params are not adjusted * fix test --- CHANGELOG.md | 1 + app/upgrades.go | 59 +++++++++++++++ app/upgrades_test.go | 119 ++++++++++++++++++++++++++++++ integration_tests/cosmoscli.py | 4 +- integration_tests/test_gov.py | 41 ++++++++-- integration_tests/test_upgrade.py | 3 + integration_tests/utils.py | 29 ++++++++ 7 files changed, 250 insertions(+), 6 deletions(-) create mode 100644 app/upgrades_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ad439e781..a5fe40588 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - [#1061](https://github.com/crypto-org-chain/chain-main/pull/1061) Integrate sdk 0.50. - [#1068](https://github.com/crypto-org-chain/chain-main/pull/1068) Upgrade ibc-go to `v8.3.2` and remove icaauth module. - [#1084](https://github.com/crypto-org-chain/chain-main/pull/1084) Add MsgModuleQuerySafe in allowed messages for ica host param. +- [#1087](https://github.com/crypto-org-chain/chain-main/pull/1087) Ensure expedited related gov params pass the basic validation. *Dec 6, 2023* diff --git a/app/upgrades.go b/app/upgrades.go index df63304a6..4c890fe75 100644 --- a/app/upgrades.go +++ b/app/upgrades.go @@ -4,12 +4,16 @@ import ( "context" "fmt" "slices" + "time" + sdkmath "cosmossdk.io/math" storetypes "cosmossdk.io/store/types" upgradetypes "cosmossdk.io/x/upgrade/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper" + govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" ) func (app *ChainApp) RegisterUpgradeHandlers(cdc codec.BinaryCodec) { @@ -29,6 +33,9 @@ func (app *ChainApp) RegisterUpgradeHandlers(cdc codec.BinaryCodec) { params.AllowMessages = append(params.AllowMessages, msg) app.ICAHostKeeper.SetParams(sdkCtx, params) } + if err := UpdateExpeditedParams(ctx, app.GovKeeper); err != nil { + return m, err + } } return m, nil }) @@ -45,3 +52,55 @@ func (app *ChainApp) RegisterUpgradeHandlers(cdc codec.BinaryCodec) { app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades)) } } + +func UpdateExpeditedParams(ctx context.Context, gov govkeeper.Keeper) error { + govParams, err := gov.Params.Get(ctx) + if err != nil { + return err + } + if len(govParams.MinDeposit) > 0 { + minDeposit := govParams.MinDeposit[0] + expeditedAmount := minDeposit.Amount.MulRaw(govv1.DefaultMinExpeditedDepositTokensRatio) + govParams.ExpeditedMinDeposit = sdk.NewCoins(sdk.NewCoin(minDeposit.Denom, expeditedAmount)) + } + threshold, err := sdkmath.LegacyNewDecFromStr(govParams.Threshold) + if err != nil { + return fmt.Errorf("invalid threshold string: %w", err) + } + expeditedThreshold, err := sdkmath.LegacyNewDecFromStr(govParams.ExpeditedThreshold) + if err != nil { + return fmt.Errorf("invalid expedited threshold string: %w", err) + } + if expeditedThreshold.LTE(threshold) { + expeditedThreshold = threshold.Mul(DefaultThresholdRatio()) + } + if expeditedThreshold.GT(sdkmath.LegacyOneDec()) { + expeditedThreshold = sdkmath.LegacyOneDec() + } + govParams.ExpeditedThreshold = expeditedThreshold.String() + if govParams.ExpeditedVotingPeriod != nil && govParams.VotingPeriod != nil && *govParams.ExpeditedVotingPeriod >= *govParams.VotingPeriod { + votingPeriod := DurationToDec(*govParams.VotingPeriod) + period := DecToDuration(DefaultPeriodRatio().Mul(votingPeriod)) + govParams.ExpeditedVotingPeriod = &period + } + if err := govParams.ValidateBasic(); err != nil { + return err + } + return gov.Params.Set(ctx, govParams) +} + +func DefaultThresholdRatio() sdkmath.LegacyDec { + return govv1.DefaultExpeditedThreshold.Quo(govv1.DefaultThreshold) +} + +func DefaultPeriodRatio() sdkmath.LegacyDec { + return DurationToDec(govv1.DefaultExpeditedPeriod).Quo(DurationToDec(govv1.DefaultPeriod)) +} + +func DurationToDec(d time.Duration) sdkmath.LegacyDec { + return sdkmath.LegacyMustNewDecFromStr(fmt.Sprintf("%f", d.Seconds())) +} + +func DecToDuration(d sdkmath.LegacyDec) time.Duration { + return time.Second * time.Duration(d.RoundInt64()) +} diff --git a/app/upgrades_test.go b/app/upgrades_test.go new file mode 100644 index 000000000..936b6fd00 --- /dev/null +++ b/app/upgrades_test.go @@ -0,0 +1,119 @@ +package app_test + +import ( + "testing" + "time" + + "cosmossdk.io/math" + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + sdk "github.com/cosmos/cosmos-sdk/types" + govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" + "github.com/crypto-org-chain/chain-main/v4/app" + "github.com/crypto-org-chain/chain-main/v4/testutil" + "github.com/stretchr/testify/suite" +) + +type AppTestSuite struct { + suite.Suite + + ctx sdk.Context + app *app.ChainApp + govParams govv1.Params +} + +func TestAppTestSuite(t *testing.T) { + suite.Run(t, new(AppTestSuite)) +} + +func (suite *AppTestSuite) SetupTest() { + checkTx := false + suite.app = testutil.Setup(checkTx, nil) + suite.ctx = suite.app.NewContext(checkTx).WithBlockHeader(tmproto.Header{Height: 1, ChainID: testutil.ChainID, Time: time.Now().UTC()}) + var err error + suite.govParams, err = suite.app.GovKeeper.Params.Get(suite.ctx) + suite.Require().NoError(err) + suite.Require().Equal(govv1.DefaultParams(), suite.govParams) +} + +func (suite *AppTestSuite) TestUpdateExpeditedParams() { + const baseDenom = "basecro" + + testCases := []struct { + name string + malleate func() + exp func(params govv1.Params) + }{ + { + name: "update ExpeditedMinDeposit with baseDenom", + malleate: func() { + suite.govParams.MinDeposit = sdk.NewCoins(sdk.NewCoin(baseDenom, math.NewInt(2000000000000))) + }, + exp: func(params govv1.Params) { + expected := sdk.NewCoins(sdk.NewCoin(suite.govParams.MinDeposit[0].Denom, suite.govParams.MinDeposit[0].Amount.MulRaw(govv1.DefaultMinExpeditedDepositTokensRatio))) + suite.Require().Equal(expected[0], params.ExpeditedMinDeposit[0]) + }, + }, + { + name: "update ExpeditedThreshold when DefaultExpeditedThreshold >= Threshold", + malleate: func() { + suite.govParams.Threshold = "0.99" + }, + exp: func(params govv1.Params) { + suite.Require().Equal(math.LegacyOneDec().String(), params.ExpeditedThreshold) + }, + }, + { + name: "update ExpeditedThreshold when DefaultExpeditedThreshold >= Threshold", + malleate: func() { + suite.govParams.Threshold = govv1.DefaultExpeditedThreshold.String() + }, + exp: func(params govv1.Params) { + expected := app.DefaultThresholdRatio().Mul(math.LegacyMustNewDecFromStr(suite.govParams.Threshold)) + suite.Require().Equal(expected.String(), params.ExpeditedThreshold) + }, + }, + { + name: "no update ExpeditedThreshold when DefaultExpeditedThreshold < Threshold", + malleate: func() { + suite.govParams.Threshold = govv1.DefaultExpeditedThreshold.Quo(math.LegacyMustNewDecFromStr("1.1")).String() + }, + exp: func(params govv1.Params) { + suite.Require().Equal(suite.govParams.ExpeditedThreshold, params.ExpeditedThreshold) + }, + }, + { + name: "update ExpeditedVotingPeriod when DefaultExpeditedPeriod >= VotingPeriod", + malleate: func() { + period := govv1.DefaultExpeditedPeriod + suite.govParams.VotingPeriod = &period + }, + exp: func(params govv1.Params) { + votingPeriod := app.DurationToDec(*suite.govParams.VotingPeriod) + expected := app.DecToDuration(app.DefaultPeriodRatio().Mul(votingPeriod)) + suite.Require().Equal(expected, *params.ExpeditedVotingPeriod) + }, + }, + { + name: "no update ExpeditedVotingPeriod when DefaultExpeditedPeriod < VotingPeriod", + malleate: func() { + period := govv1.DefaultExpeditedPeriod + 1 + suite.govParams.VotingPeriod = &period + }, + exp: func(params govv1.Params) { + suite.Require().Equal(*suite.govParams.ExpeditedVotingPeriod, *params.ExpeditedVotingPeriod) + }, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.SetupTest() + tc.malleate() + suite.Require().NoError(suite.app.GovKeeper.Params.Set(suite.ctx, suite.govParams)) + suite.Require().NoError(app.UpdateExpeditedParams(suite.ctx, suite.app.GovKeeper)) + params, err := suite.app.GovKeeper.Params.Get(suite.ctx) + suite.Require().NoError(err) + tc.exp(params) + }) + } +} diff --git a/integration_tests/cosmoscli.py b/integration_tests/cosmoscli.py index c7bb5fa59..ac42d9567 100644 --- a/integration_tests/cosmoscli.py +++ b/integration_tests/cosmoscli.py @@ -298,7 +298,7 @@ def query_params(self, mod): "node": self.node_rpc, "output": "json", } - return json.loads( + res = json.loads( self.raw( "q", mod, @@ -306,6 +306,8 @@ def query_params(self, mod): **kwargs, ) ) + res = res.get("params") or res + return res class ClusterCLI(cluster.ClusterCLI): diff --git a/integration_tests/test_gov.py b/integration_tests/test_gov.py index e9b8e1248..e2507b4fe 100644 --- a/integration_tests/test_gov.py +++ b/integration_tests/test_gov.py @@ -6,6 +6,8 @@ from .utils import ( approve_proposal, + assert_gov_params, + get_expedited_params, get_proposal_id, module_address, wait_for_block, @@ -222,6 +224,35 @@ def test_inherit_vote(cluster, tmp_path): def test_host_enabled(cluster, tmp_path): cli = cluster.cosmos_cli() + param0 = cli.query_params("gov") + param1 = get_expedited_params(param0) + # governance module account as signer + authority = module_address("gov") + proposal = tmp_path / "proposal.json" + type = "/cosmos.gov.v1.MsgUpdateParams" + deposit = "0.1cro" + proposal_src = { + "title": "title", + "summary": "summary", + "deposit": deposit, + "messages": [ + { + "@type": type, + "authority": authority, + "params": { + **param0, + **param1, + }, + } + ], + } + proposal.write_text(json.dumps(proposal_src)) + rsp = cli.submit_gov_proposal(proposal, from_="community") + assert rsp["code"] == 0, rsp["raw_log"] + approve_proposal(cluster, rsp, msg=f",{type}") + print("check params have been updated now") + assert_gov_params(cli, param0) + p = cli.query_host_params() assert p["host_enabled"] p["host_enabled"] = False @@ -236,7 +267,7 @@ def test_host_enabled(cluster, tmp_path): "params": p, } ], - "deposit": "10000000basecro", + "deposit": deposit, "title": "title", "summary": "summary", } @@ -255,9 +286,9 @@ def test_gov_voting(cluster, tmp_path): """ cli = cluster.cosmos_cli() p = cli.query_params("gov") - assert p["params"]["voting_period"] == "10s" + assert p["voting_period"] == "10s" updated = "3m36s" - p["params"]["voting_period"] = updated + p["voting_period"] = updated proposal = tmp_path / "proposal.json" authority = module_address("gov") type = "/cosmos.gov.v1.MsgUpdateParams" @@ -266,7 +297,7 @@ def test_gov_voting(cluster, tmp_path): { "@type": type, "authority": authority, - "params": p["params"], + "params": p, } ], "deposit": "10000000basecro", @@ -278,4 +309,4 @@ def test_gov_voting(cluster, tmp_path): assert rsp["code"] == 0, rsp["raw_log"] approve_proposal(cluster, rsp, msg=f",{type}") p = cli.query_params("gov") - assert p["params"]["voting_period"] == updated + assert p["voting_period"] == updated diff --git a/integration_tests/test_upgrade.py b/integration_tests/test_upgrade.py index 5f4c543d9..32e309566 100644 --- a/integration_tests/test_upgrade.py +++ b/integration_tests/test_upgrade.py @@ -13,6 +13,7 @@ from .utils import ( approve_proposal, + assert_gov_params, cluster_fixture, wait_for_block, wait_for_block_time, @@ -428,10 +429,12 @@ def assert_commission(adr, expected): upgrade(cluster, "v4.3.0", target_height) target_height = cluster.block_height() + 15 + gov_param = cli.query_params("gov") upgrade(cluster, "v5.0", target_height, broadcast_mode="sync") cli = cluster.cosmos_cli() with pytest.raises(AssertionError): cli.query_params("icaauth") + assert_gov_params(cli, gov_param) def test_cancel_upgrade(cluster): diff --git a/integration_tests/utils.py b/integration_tests/utils.py index bfca3803e..8e1e1fe25 100644 --- a/integration_tests/utils.py +++ b/integration_tests/utils.py @@ -5,6 +5,7 @@ import sys import time from datetime import timedelta +from decimal import Decimal import bech32 from dateutil.parser import isoparse @@ -587,3 +588,31 @@ def wait_for_fn(name, fn, *, timeout=240, interval=1): time.sleep(interval) else: raise TimeoutError(f"wait for {name} timeout") + + +def get_expedited_params(param): + min_deposit = param["min_deposit"][0] + voting_period = param["voting_period"] + tokens_ratio = 5 + threshold_ratio = 1.334 + period_ratio = 0.5 + expedited_threshold = float(param["threshold"]) * threshold_ratio + expedited_threshold = Decimal(f"{expedited_threshold}") + expedited_voting_period = int(int(voting_period[:-1]) * period_ratio) + return { + "expedited_min_deposit": [ + { + "denom": min_deposit["denom"], + "amount": str(int(min_deposit["amount"]) * tokens_ratio), + } + ], + "expedited_threshold": f"{expedited_threshold:.18f}", + "expedited_voting_period": f"{expedited_voting_period}s", + } + + +def assert_gov_params(cli, old_param): + param = cli.query_params("gov") + expedited_param = get_expedited_params(old_param) + for key, value in expedited_param.items(): + assert param[key] == value, param