Problem: expedited related gov params are not adjusted

CHANGELOG.md

@@ -22,6 +22,7 @@
 * [#1611](https://github.com/crypto-org-chain/cronos/pull/1611) Fix multisig account failed on threshold encode after send tx.
 * [#1617](https://github.com/crypto-org-chain/cronos/pull/1617) Fix unsuppored sign mode SIGN_MODE_TEXTUAL for bank transfer.
 * [#1621](https://github.com/crypto-org-chain/cronos/pull/1621) Update ethermint to the fix of broken opBlockhash and tx validation.
+* [#1623](https://github.com/crypto-org-chain/cronos/pull/1623) Ensure expedited related gov params pass the basic validation.
 
 *Sep 13, 2024*
 

app/app.go

@@ -954,7 +954,7 @@ func New(
 
 	// RegisterUpgradeHandlers is used for registering any on-chain upgrades.
 	// Make sure it's called after `app.mm` and `app.configurator` are set.
-	app.RegisterUpgradeHandlers(app.appCodec, app.IBCKeeper.ClientKeeper)
+	app.RegisterUpgradeHandlers(app.appCodec)
 
 	// add test gRPC service for testing gRPC queries in isolation
 	// testdata.RegisterQueryServer(app.GRPCQueryRouter(), testdata.QueryImpl{})

app/upgrades.go

@@ -3,18 +3,21 @@ package app
 import (
 	"context"
 	"fmt"
+	"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"
 	icahosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
-	clientkeeper "github.com/cosmos/ibc-go/v8/modules/core/02-client/keeper"
 	evmtypes "github.com/evmos/ethermint/x/evm/types"
 )
 
-func (app *App) RegisterUpgradeHandlers(cdc codec.BinaryCodec, clientKeeper clientkeeper.Keeper) {
+func (app *App) RegisterUpgradeHandlers(cdc codec.BinaryCodec) {
 	planName := "v1.4"
 	app.UpgradeKeeper.SetUpgradeHandler(planName, func(ctx context.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
 		m, err := app.ModuleManager.RunMigrations(ctx, app.configurator, fromVM)
@@ -32,6 +35,9 @@ func (app *App) RegisterUpgradeHandlers(cdc codec.BinaryCodec, clientKeeper clie
 			if err := app.EvmKeeper.SetParams(sdkCtx, evmParams); err != nil {
 				return m, err
 			}
+			if err := UpdateExpeditedParams(ctx, app.GovKeeper); err != nil {
+				return m, err
+			}
 		}
 		return m, nil
 	})
@@ -51,3 +57,55 @@ func (app *App) RegisterUpgradeHandlers(cdc codec.BinaryCodec, clientKeeper clie
 		}
 	}
 }
+
+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,120 @@
+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/cronos/v2/app"
+	"github.com/evmos/ethermint/crypto/ethsecp256k1"
+	"github.com/stretchr/testify/suite"
+)
+
+type AppTestSuite struct {
+	suite.Suite
+
+	ctx       sdk.Context
+	app       *app.App
+	govParams govv1.Params
+}
+
+func TestAppTestSuite(t *testing.T) {
+	suite.Run(t, new(AppTestSuite))
+}
+
+func (suite *AppTestSuite) SetupTest() {
+	checkTx := false
+	privKey, err := ethsecp256k1.GenerateKey()
+	suite.Require().NoError(err)
+	suite.app = app.Setup(suite.T(), sdk.AccAddress(privKey.PubKey().Address()).String())
+	suite.ctx = suite.app.NewContext(checkTx).WithBlockHeader(tmproto.Header{Height: 1, ChainID: app.TestAppChainID, Time: time.Now().UTC()})
+	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 = "basetcro"
+
+	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

@@ -1110,7 +1110,7 @@ def query_gravity_params(self):
         return self.query_params("gravity")
 
     def query_params(self, module="cronos", **kwargs):
-        return json.loads(
+        res = json.loads(
             self.raw(
                 "query",
                 module,
@@ -1119,6 +1119,8 @@ def query_params(self, module="cronos", **kwargs):
                 **kwargs,
             )
         )
+        res = res.get("params") or res
+        return res
 
     def query_signer_set_txs(self):
         return json.loads(

integration_tests/gravity_utils.py

@@ -116,7 +116,7 @@ def prepare_gravity(custom_cronos, custom_geth):
     send_transaction(w3, {"to": admin.address, "value": 10**17}, KEYS["validator"])
 
     # deploy gravity contract to geth
-    gravity_id = cli.query_gravity_params()["params"]["gravity_id"]
+    gravity_id = cli.query_gravity_params()["gravity_id"]
     signer_set = cli.query_latest_signer_set_tx()["signer_set"]["signers"]
     powers = [int(signer["power"]) for signer in signer_set]
     threshold = int(2**32 * 0.66)  # gravity normalize the power to [0, 2**32]

integration_tests/test_basic.py

@@ -1,4 +1,3 @@
-import hashlib
 import json
 import subprocess
 import time
@@ -12,28 +11,29 @@
 from hexbytes import HexBytes
 from pystarport import cluster, ports
 
-from .cosmoscli import CosmosCLI
+from .cosmoscli import CosmosCLI, module_address
 from .network import Geth
 from .utils import (
     ADDRS,
     CONTRACTS,
     KEYS,
     Greeter,
     RevertTestContract,
-    approve_proposal,
+    assert_gov_params,
     build_batch_tx,
     contract_address,
     contract_path,
     deploy_contract,
     derive_new_account,
-    eth_to_bech32,
+    get_expedited_params,
     get_receipts_by_block,
     get_sync_info,
     modify_command_in_supervisor_config,
     send_transaction,
     send_txs,
     sign_transaction,
     submit_any_proposal,
+    submit_gov_proposal,
     w3_wait_for_block,
     wait_for_block,
     wait_for_new_blocks,
@@ -43,31 +43,43 @@
 
 def test_ica_enabled(cronos, tmp_path):
     cli = cronos.cosmos_cli()
+    param0 = cli.query_params("gov")
+    param1 = get_expedited_params(param0)
+    # governance module account as signer
+    authority = module_address("gov")
+    submit_gov_proposal(
+        cronos,
+        tmp_path,
+        messages=[
+            {
+                "@type": "/cosmos.gov.v1.MsgUpdateParams",
+                "authority": authority,
+                "params": {
+                    **param0,
+                    **param1,
+                },
+            }
+        ],
+    )
+    assert_gov_params(cli, param0)
+
     p = cli.query_ica_params()
     assert p["controller_enabled"]
     p["controller_enabled"] = False
-    proposal = tmp_path / "proposal.json"
-    # governance module account as signer
-    data = hashlib.sha256("gov".encode()).digest()[:20]
-    signer = eth_to_bech32(data)
     type = "/ibc.applications.interchain_accounts.controller.v1.MsgUpdateParams"
-    proposal_src = {
-        "messages": [
+    submit_gov_proposal(
+        cronos,
+        tmp_path,
+        messages=[
             {
                 "@type": type,
-                "signer": signer,
+                "signer": authority,
                 "params": p,
             }
         ],
-        "deposit": "1basetcro",
-        "title": "title",
-        "summary": "summary",
-    }
-    proposal.write_text(json.dumps(proposal_src))
-    rsp = cli.submit_gov_proposal(proposal, from_="community")
-    assert rsp["code"] == 0, rsp["raw_log"]
-    approve_proposal(cronos, rsp["events"])
-    print("check params have been updated now")
+        deposit="5basetcro",
+        expedited=True,
+    )
     p = cli.query_ica_params()
     assert not p["controller_enabled"]
 
@@ -912,30 +924,22 @@ def test_submit_send_enabled(cronos, tmp_path):
     cli = cronos.cosmos_cli()
     denoms = ["basetcro", "stake"]
     assert len(cli.query_bank_send(*denoms)) == 0, "should be empty"
-    proposal = tmp_path / "proposal.json"
-    # governance module account as signer
-    signer = "crc10d07y265gmmuvt4z0w9aw880jnsr700jdufnyd"
     send_enable = [
         {"denom": "basetcro"},
         {"denom": "stake", "enabled": True},
     ]
-    proposal_src = {
-        "messages": [
+    authority = module_address("gov")
+    submit_gov_proposal(
+        cronos,
+        tmp_path,
+        messages=[
             {
                 "@type": "/cosmos.bank.v1beta1.MsgSetSendEnabled",
-                "authority": signer,
+                "authority": authority,
                 "sendEnabled": send_enable,
             }
         ],
-        "deposit": "1basetcro",
-        "title": "title",
-        "summary": "summary",
-    }
-    proposal.write_text(json.dumps(proposal_src))
-    rsp = cli.submit_gov_proposal(proposal, from_="community")
-    assert rsp["code"] == 0, rsp["raw_log"]
-    approve_proposal(cronos, rsp["events"])
-    print("check params have been updated now")
+    )
     assert cli.query_bank_send(*denoms) == send_enable