Problem: expedited related gov params are not adjusted (#1087)

* Problem: expedited related gov params are not adjusted

* fix test

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*
 

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())
+}

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)
+		})
+	}
+}

integration_tests/cosmoscli.py

@@ -298,14 +298,16 @@ def query_params(self, mod):
             "node": self.node_rpc,
             "output": "json",
         }
-        return json.loads(
+        res = json.loads(
             self.raw(
                 "q",
                 mod,
                 "params",
                 **kwargs,
             )
         )
+        res = res.get("params") or res
+        return res
 
 
 class ClusterCLI(cluster.ClusterCLI):

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

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):

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