Added rate limits in upgrade (#4340)

* added rate limits in upgrade

* added upgrade test

* refactor rate limiting tests and added rate limits pre-upgrade

* lint

* goimports

* experiments for E2E

* post merge lints

* added RL before upgrade

* just get the latest contract

* experiments

* updated wasm file

* remove unnecessary prints

* bad check

* experiments without e2e

* removed unnecessary fmt

* param space initialization for rate limiting (#4360)

* param space initialization for rate limiting

* lint

* better test

* properly initialize empty params

* fix typo

* adding RL to E2E

* removing RL again from E2E

* debug

* genesis

* genesis 2

* add genesis and solve params bug

* uncomment setup rate limiting

* added configurable gov module

* clean up

* clean up and genesis test

* upgrade handler

* genesis clean up

* clean up e2e

* wire querier

* route fix

* fix router

* debug things

* fix init genesis bug

* push fix

* lint

* fix

* clean up upgrades

* testing propper params after upgrade in E2E

* "properly" unmarshaling the parm

* goimports

* allowing for types other than objects in cosmwasm queries

* fix check for param reset

* remove unnecessary prints

* compiled with the proper vesion of workspace optimizer

* Revert "compiled with the proper vesion of workspace optimizer"

This reverts commit ab5cf6c.

* added length check

* params experiment for test with state-export

---------

Co-authored-by: Roman <[email protected]>
Co-authored-by: Roman <[email protected]>

app/keepers/keepers.go

@@ -504,15 +504,13 @@ func (appKeepers *AppKeepers) WireICS20PreWasmKeeper(
 	)
 
 	// ChannelKeeper wrapper for rate limiting SendPacket(). The wasmKeeper needs to be added after it's created
-	rateLimitingParams := appKeepers.GetSubspace(ibcratelimittypes.ModuleName)
-	rateLimitingParams = rateLimitingParams.WithKeyTable(ibcratelimittypes.ParamKeyTable())
 	rateLimitingICS4Wrapper := ibcratelimit.NewICS4Middleware(
 		appKeepers.HooksICS4Wrapper,
 		appKeepers.AccountKeeper,
 		// wasm keeper we set later.
 		nil,
 		appKeepers.BankKeeper,
-		rateLimitingParams,
+		appKeepers.GetSubspace(ibcratelimittypes.ModuleName),
 	)
 	appKeepers.RateLimitingICS4Wrapper = &rateLimitingICS4Wrapper
 

app/keepers/modules.go

@@ -34,7 +34,7 @@ import (
 	downtimemodule "github.com/osmosis-labs/osmosis/v14/x/downtime-detector/module"
 	"github.com/osmosis-labs/osmosis/v14/x/epochs"
 	"github.com/osmosis-labs/osmosis/v14/x/gamm"
-	ibc_rate_limit "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit"
+	"github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/ibcratelimitmodule"
 	"github.com/osmosis-labs/osmosis/v14/x/incentives"
 	"github.com/osmosis-labs/osmosis/v14/x/lockup"
 	"github.com/osmosis-labs/osmosis/v14/x/mint"
@@ -103,6 +103,6 @@ var AppModuleBasics = []module.AppModuleBasic{
 	icq.AppModuleBasic{},
 	ica.AppModuleBasic{},
 	ibc_hooks.AppModuleBasic{},
-	ibc_rate_limit.AppModuleBasic{},
+	ibcratelimitmodule.AppModuleBasic{},
 	router.AppModuleBasic{},
 }

app/modules.go

@@ -64,6 +64,8 @@ import (
 	epochstypes "github.com/osmosis-labs/osmosis/v14/x/epochs/types"
 	"github.com/osmosis-labs/osmosis/v14/x/gamm"
 	gammtypes "github.com/osmosis-labs/osmosis/v14/x/gamm/types"
+	"github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/ibcratelimitmodule"
+	ibcratelimittypes "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/types"
 	"github.com/osmosis-labs/osmosis/v14/x/incentives"
 	incentivestypes "github.com/osmosis-labs/osmosis/v14/x/incentives/types"
 	"github.com/osmosis-labs/osmosis/v14/x/lockup"
@@ -171,6 +173,7 @@ func appModules(
 		),
 		tokenfactory.NewAppModule(*app.TokenFactoryKeeper, app.AccountKeeper, app.BankKeeper),
 		valsetprefmodule.NewAppModule(appCodec, *app.ValidatorSetPreferenceKeeper),
+		ibcratelimitmodule.NewAppModule(*app.RateLimitingICS4Wrapper),
 		ibc_hooks.NewAppModule(app.AccountKeeper),
 		icq.NewAppModule(*app.AppKeepers.ICQKeeper),
 	}
@@ -252,6 +255,7 @@ func OrderInitGenesis(allModuleNames []string) []string {
 		lockuptypes.ModuleName,
 		authz.ModuleName,
 		concentratedliquiditytypes.ModuleName,
+		ibcratelimittypes.ModuleName,
 		// wasm after ibc transfer
 		wasm.ModuleName,
 		// ibc_hooks after auth keeper

app/upgrades/v15/export_test.go

@@ -1,7 +1,10 @@
 package v15
 
 import (
+	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
+	ibcratelimit "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit"
 	icqkeeper "github.com/strangelove-ventures/async-icq/v4/keeper"
 
 	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
@@ -21,3 +24,7 @@ func RegisterOsmoIonMetadata(ctx sdk.Context, bankKeeper bankkeeper.Keeper) {
 func SetICQParams(ctx sdk.Context, icqKeeper *icqkeeper.Keeper) {
 	setICQParams(ctx, icqKeeper)
 }
+
+func SetRateLimits(ctx sdk.Context, accountKeeper *authkeeper.AccountKeeper, rateLimitingICS4Wrapper *ibcratelimit.ICS4Wrapper, wasmKeeper *wasmkeeper.Keeper) {
+	setRateLimits(ctx, accountKeeper, rateLimitingICS4Wrapper, wasmKeeper)
+}

app/upgrades/v15/upgrade_test.go

@@ -1,9 +1,19 @@
 package v15_test
 
 import (
+	"fmt"
+	"os"
 	"reflect"
 	"testing"
 
+	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
+	wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+	transfertypes "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types"
+
+	ibcratelimittypes "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/types"
+
 	gamm "github.com/osmosis-labs/osmosis/v14/x/gamm/keeper"
 
 	"github.com/stretchr/testify/suite"
@@ -105,3 +115,47 @@ func (suite *UpgradeTestSuite) TestSetICQParams() {
 	suite.Require().True(suite.App.ICQKeeper.IsHostEnabled(suite.Ctx))
 	suite.Require().Len(suite.App.ICQKeeper.GetAllowQueries(suite.Ctx), 63)
 }
+
+func (suite *UpgradeTestSuite) TestSetRateLimits() {
+	suite.SetupTest() // reset
+	accountKeeper := suite.App.AccountKeeper
+	govModule := accountKeeper.GetModuleAddress(govtypes.ModuleName)
+
+	code, err := os.ReadFile("../v13/rate_limiter.wasm")
+	suite.Require().NoError(err)
+	contractKeeper := wasmkeeper.NewGovPermissionKeeper(suite.App.WasmKeeper)
+	instantiateConfig := wasmtypes.AccessConfig{Permission: wasmtypes.AccessTypeOnlyAddress, Address: govModule.String()}
+	codeID, _, err := contractKeeper.Create(suite.Ctx, govModule, code, &instantiateConfig)
+	suite.Require().NoError(err)
+	transferModule := accountKeeper.GetModuleAddress(transfertypes.ModuleName)
+	initMsgBz := []byte(fmt.Sprintf(`{
+           "gov_module":  "%s",
+           "ibc_module":"%s",
+           "paths": []
+        }`,
+		govModule, transferModule))
+
+	addr, _, err := contractKeeper.Instantiate(suite.Ctx, codeID, govModule, govModule, initMsgBz, "rate limiting contract", nil)
+	suite.Require().NoError(err)
+	addrStr, err := sdk.Bech32ifyAddressBytes("osmo", addr)
+	suite.Require().NoError(err)
+	params, err := ibcratelimittypes.NewParams(addrStr)
+	suite.Require().NoError(err)
+	paramSpace, ok := suite.App.ParamsKeeper.GetSubspace(ibcratelimittypes.ModuleName)
+	suite.Require().True(ok)
+	paramSpace.SetParamSet(suite.Ctx, &params)
+
+	// system under test.
+	v15.SetRateLimits(suite.Ctx, accountKeeper, suite.App.RateLimitingICS4Wrapper, suite.App.WasmKeeper)
+
+	state, err := suite.App.WasmKeeper.QuerySmart(suite.Ctx, addr, []byte(`{"get_quotas": {"channel_id": "any", "denom": "ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2"}}`))
+	suite.Require().Greaterf(len(state), 0, "state should not be empty")
+
+	state, err = suite.App.WasmKeeper.QuerySmart(suite.Ctx, addr, []byte(`{"get_quotas": {"channel_id": "any", "denom": "ibc/D189335C6E4A68B513C10AB227BF1C1D38C746766278BA3EEB4FB14124F1D858"}}`))
+	suite.Require().Greaterf(len(state), 0, "state should not be empty")
+
+	// This is the last one. If the others failed the upgrade would've panicked before adding this one
+	state, err = suite.App.WasmKeeper.QuerySmart(suite.Ctx, addr, []byte(`{"get_quotas": {"channel_id": "any", "denom": "ibc/E6931F78057F7CC5DA0FD6CEF82FF39373A6E0452BF1FD76910B93292CF356C1"}}`))
+	suite.Require().Greaterf(len(state), 0, "state should not be empty")
+
+}

app/upgrades/v15/upgrades.go

@@ -1,15 +1,23 @@
 package v15
 
 import (
+	packetforwardtypes "github.com/strangelove-ventures/packet-forward-middleware/v4/router/types"
+
+	poolmanagertypes "github.com/osmosis-labs/osmosis/v14/x/poolmanager/types"
+
+	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
+	authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
-	"github.com/osmosis-labs/osmosis/v14/wasmbinding"
 	icqkeeper "github.com/strangelove-ventures/async-icq/v4/keeper"
 	icqtypes "github.com/strangelove-ventures/async-icq/v4/types"
-	packetforwardtypes "github.com/strangelove-ventures/packet-forward-middleware/v4/router/types"
 
-	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/osmosis-labs/osmosis/v14/wasmbinding"
+	ibcratelimit "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit"
+	ibcratelimittypes "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/types"
 
 	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
 
@@ -18,7 +26,6 @@ import (
 	"github.com/osmosis-labs/osmosis/v14/app/upgrades"
 	gammkeeper "github.com/osmosis-labs/osmosis/v14/x/gamm/keeper"
 	"github.com/osmosis-labs/osmosis/v14/x/poolmanager"
-	poolmanagertypes "github.com/osmosis-labs/osmosis/v14/x/poolmanager/types"
 )
 
 func CreateUpgradeHandler(
@@ -43,10 +50,17 @@ func CreateUpgradeHandler(
 		// See RunMigrations() for details.
 		fromVM[poolmanagertypes.ModuleName] = 0
 
+		//  N.B.: this is done to avoid initializing genesis for ibcratelimit module.
+		// Otherwise, it would overwrite migrations with InitGenesis().
+		// See RunMigrations() for details.
+		fromVM[ibcratelimittypes.ModuleName] = 0
+
 		// Metadata for uosmo and uion were missing prior to this upgrade.
 		// They are added in this upgrade.
 		registerOsmoIonMetadata(ctx, keepers.BankKeeper)
 
+		setRateLimits(ctx, keepers.AccountKeeper, keepers.RateLimitingICS4Wrapper, keepers.WasmKeeper)
+
 		return mm.RunMigrations(ctx, configurator, fromVM)
 	}
 }
@@ -59,6 +73,92 @@ func setICQParams(ctx sdk.Context, icqKeeper *icqkeeper.Keeper) {
 	icqKeeper.SetParams(ctx, icqparams)
 }
 
+func setRateLimits(ctx sdk.Context, accountKeeper *authkeeper.AccountKeeper, rateLimitingICS4Wrapper *ibcratelimit.ICS4Wrapper, wasmKeeper *wasmkeeper.Keeper) {
+	govModule := accountKeeper.GetModuleAddress(govtypes.ModuleName)
+	contractKeeper := wasmkeeper.NewGovPermissionKeeper(wasmKeeper)
+
+	paths := []string{
+		`{"add_path": {"channel_id": "any", "denom": "ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2",
+          "quotas":
+            [
+              {"name":"ATOM-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"ATOM-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/D189335C6E4A68B513C10AB227BF1C1D38C746766278BA3EEB4FB14124F1D858",
+          "quotas":
+            [
+              {"name":"USDC-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"USDC-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/D1542AA8762DB13087D8364F3EA6509FD6F009A34F00426AF9E4F9FA85CBBF1F",
+          "quotas":
+            [
+              {"name":"WBTC-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"WBTC-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/EA1D43981D5C9A1C4AAEA9C23BB1D4FA126BA9BC7020A25E0AE4AA841EA25DC5",
+          "quotas":
+            [
+              {"name":"WETH-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"WETH-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/6AE98883D4D5D5FF9E50D7130F1305DA2FFA0C652D1DD9C123657C6B4EB2DF8A",
+          "quotas":
+            [
+              {"name":"EVMOS-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"EVMOS-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/987C17B11ABC2B20019178ACE62929FE9840202CE79498E29FE8E5CB02B7C0A4",
+          "quotas":
+            [
+              {"name":"STARS-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"STARS-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7",
+          "quotas":
+            [
+              {"name":"DAI-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"DAI-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/46B44899322F3CD854D2D46DEEF881958467CDD4B3B10086DA49296BBED94BED",
+          "quotas":
+            [
+              {"name":"JUNO-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"JUNO-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+		`{"add_path": {"channel_id": "any", "denom": "ibc/E6931F78057F7CC5DA0FD6CEF82FF39373A6E0452BF1FD76910B93292CF356C1",
+          "quotas":
+            [
+              {"name":"CRO-DAY","duration":86400,"send_recv":[30,30]},
+              {"name":"CRO-WEEK","duration":604800,"send_recv":[60,60]}
+            ]
+          }}`,
+	}
+
+	contract := rateLimitingICS4Wrapper.GetContractAddress(ctx)
+	if contract == "" {
+		panic("rate limiting contract not set")
+	}
+	rateLimitingContract, err := sdk.AccAddressFromBech32(contract)
+	if err != nil {
+		panic("contract address improperly formatted")
+	}
+	for _, denom := range paths {
+		_, err := contractKeeper.Execute(ctx, rateLimitingContract, govModule, []byte(denom), nil)
+		if err != nil {
+			panic(err)
+		}
+	}
+}
+
 func migrateNextPoolId(ctx sdk.Context, gammKeeper *gammkeeper.Keeper, poolmanagerKeeper *poolmanager.Keeper) {
 	// N.B: pool id in gamm is to be deprecated in the future
 	// Instead,it is moved to poolmanager.

proto/osmosis/ibc-rate-limit/v1beta1/genesis.proto

@@ -0,0 +1,15 @@
+syntax = "proto3";
+package osmosis.ibcratelimit.v1beta1;
+
+import "gogoproto/gogo.proto";
+import "cosmos_proto/cosmos.proto";
+import "google/protobuf/any.proto";
+import "osmosis/ibc-rate-limit/v1beta1/params.proto";
+
+option go_package = "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/types";
+
+// GenesisState defines the ibc-rate-limit module's genesis state.
+message GenesisState {
+  // params are all the parameters of the module
+  Params params = 1 [ (gogoproto.nullable) = false ];
+}

proto/osmosis/ibc-rate-limit/v1beta1/query.proto

@@ -6,22 +6,22 @@ import "google/api/annotations.proto";
 import "cosmos/base/query/v1beta1/pagination.proto";
 import "osmosis/ibc-rate-limit/v1beta1/params.proto";
 
-option go_package = "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/types";
+option go_package = "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/client/queryproto";
 
 // Query defines the gRPC querier service.
 service Query {
   // Params defines a gRPC query method that returns the ibc-rate-limit module's
   // parameters.
-  rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {
+  rpc Params(ParamsRequest) returns (ParamsResponse) {
     option (google.api.http).get = "/osmosis/ibc-rate-limit/v1beta1/params";
   }
 }
 
-// QueryParamsRequest is the request type for the Query/Params RPC method.
-message QueryParamsRequest {}
+// ParamsRequest is the request type for the Query/Params RPC method.
+message ParamsRequest {}
 
-// QueryParamsResponse is the response type for the Query/Params RPC method.
-message QueryParamsResponse {
+// aramsResponse is the response type for the Query/Params RPC method.
+message ParamsResponse {
   // params defines the parameters of the module.
   Params params = 1 [ (gogoproto.nullable) = false ];
 }

proto/osmosis/ibc-rate-limit/v1beta1/query.yml

@@ -0,0 +1,10 @@
+keeper: 
+  path: "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit"
+  struct: "Keeper"
+client_path: "github.com/osmosis-labs/osmosis/v14/x/ibc-rate-limit/client"
+queries:
+  Params:
+    proto_wrapper:
+      query_func: "k.GetParams"
+    cli:
+      cmd: "GetParams"