Rate limit - Cleaner tests (osmosis-labs#3183)

* improved testing framework

* can test both send and recv for success and failure

* cleanner testing framework

* added contract instantiation

* working wasm integration

* added params for contract config

* extracted param registration

* active rate limiting

* calculating channel value

* cleaner tests

* fix issue with epochs

* fixed tests

* testing rate limit reset

* linting

* added receive middleware

* added test for non-configured channel

* make format

* Revert "make format"

This reverts commit 9ffdc37.

* only applying format to ibc-rate-limit

* applying fmt to app.go

* added gov_module and changed no-quota default to "allow all"

* added asymetric quotas

* moved getters to modules.go

* initial work to support multiple quotas

* added multiple quotas

* small fixes

* reordered imports

* added management messages

* reorganized management messages and experimenting with e2e testing

* commenting out test configuration test for now

* added query

* added flow unit test

* cleanup

* added AddChannel tests

* format

* test values are properly stored

* testing remove channel

* some more rate limiting tests

* moved tests about test setup to the right place

* fixed params

* merged main

* running gofumpt

* added ibc-rate-limiting contract

* added ibc-rate-limit middleware

* added chain integration and tests

* reverted change to match merged branch in main (osmosis-labs#2341 instead of osmosis-labs#2274)

* added cosmwasm workflow

* added a migrate message

* added some doc comments to the state

* added doc comments

* fixed dependency after merging osmosis-labs/cosmos-sdk#312

* added migration msg

* added workflow

* experimenting with better workflow

* added missing $

* using env

* Update x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs

Co-authored-by: Dev Ojha <[email protected]>

* using stable for clippy

* removed gitkeep

* using the minimal profile for clippy

* experimenting with cache

* removed target from lints

* cleaner matrix?

* COmments & questions

* debugging

* more debugging

* debug faster

* quick cache debug

* typo

* quick workflow check

* working tests with optimization

* testing artifacts

* split the wasm target into its own step

* artifacts without slash

* full working tests

* clippy fixes

* workflow without test data checks and clippy fixes

* renamed CHANNEL_FLOWS

* renaming and code clenaup

* more renames

* renames and code fixes

* reordered imports

* cargo fmt

* added danom tracking

* cleanup

* refactoring

* changes to the expiration logic so that balances are calculated based on the direction (as suggested by @ValarDragon)

* slightly slower but considerably cleanner

* cleanup attributes and removed redundancy when not testing

* update to edition 2021

* added comments explaining the tests

* removed .beaker

* unified gitignore

* removed second gitignore

* better doc comments

* spelling

* spelling

* added channel value cache

* updated the middlware to use the new contract interface

* update middleware to match new contract interface

* added missing updates

* updated dependencies

* added missing helpers

* go.mod changes shouldn't be in this branch

* Revert "go.mod changes shouldn't be in this branch"

This reverts commit f8b972a.

* moved send and receive to sudo

* reorganizing

* calling the contract via sudo

* lint

* removed gitkeep

* using sudo instead of execute

* cleaned up and updated contract and integration

* updated x86 test wasm file

* fixed bad print

* storing and instantiating the contract

* setting up E2E tests for ibc rate limits

* fixed proposal. Now just have to get the math right and cleanup

* Using the supply for the channel value

* experimenting with e2e tests

* passing the contract keeper instead of instantiating it each time

* changes from code review

* added contract from main and changes to the middleware from code review

* using the correct bank supply method

* debugging issues with e2e tests. Everything works after one interaction from the cli, but not before

* updated dependency to match latest sdk form (now that the changes to this repo have been applied)

* working E2E test for rate limiting

* added e2e tests and changes from code review

* removed debug logs

* remove debug logs

* updated test to also use GetSupplyWithOffset

* using correct GetSupplyWithOffset method

* using correct GetSupplyWithOffset method

* lint

* removed e2e from this branch as it's not doing anything without the integration

* lint

* tests fail on CI because of "inactive" proposal. Is the deposit the issue?

* remove rate limiting after the test so it doesn't interfeer with the other tests

* using standard proposals instead of expedited

* added packet reverts on unsuccessful acks and timeouts

* lint

* ran gofumpt

* lint

* added undo to the contract

* integrating undo

* updated contract with x86 wasm file

* added undo for sent packages when they are rejected bia timeout or a bad ack

* added a readme

* markdown lint

* added readme

* abstracted params

* better params and param tests

* using a helper function instead of returning from the test

* updated contract to allow for undo

* added undo, readme, and cleanup based on reviews

* updated wasm file with x86 version

* using string params in e2e tests

* updated to v12

* removed unnecessary keeper

* only exposing what's needed

* refactoring

* updated types

* added shell history to gitignore

* adding only one wasm file to the codebase

* remove test for same wasm files. No longer needed.

* refactor based on code review

* removed integration tests as they won't pass without integration

* added params unit test

* reorganized tests

* reorganizing tests

* refactoring

* added address length limit

* added tests and fixed lack of return

* remove tests from bad merge

* remove from bad merge again

* comment

* test helpers for cosmwasm contracts

* added helpers for ibctesting

* comments

* removed unnecessary txConfig

* fixed typos

* clearer comment

* using second helper function for ExportGenesis

* added new wasm file

* updated the contract to cosmwasm 1.1 and Uint256 for amounts

* Fixed send with ibc assets. Better tests and error messages

* updated contract with x86 version

* gofumpt

* fixed clippy errors

* using the escrowed value as the channel value for native tokens

* gofumpt

* update fail string

* initial experiments with moving the calculations into the contract

* initial experiments with using the packet inside the contract

* improved tests. Experiments with packet in the contract.

* original contract

* cleaner tests

* more test cleanup

* cleaner tests

* cleanup

* align values from the tests and the contract

* fixed amounts for receive and cleanup code

* removed redundant wrapping logic

* adaped failed send test to the new testing abstractions

* only manipulate time on chain A

* remove commented out block

* changed lints to stable so they change less often

* gofumpt

* update channel value tests

* added x86 version of the contract for ci

* remove lint type that doesn't exist on stable

Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Dev Ojha <[email protected]>

.github/workflows/contracts.yml

@@ -82,11 +82,11 @@ jobs:
           path: ${{ matrix.contract.workdir }}${{ matrix.contract.build }}
           retention-days: 1
 
-#      - name: Check Test Data
-#        working-directory: ${{ matrix.contract.workdir }}
-#        if: ${{ matrix.contract.output != null }}
-#        run: >
-#          diff ${{ matrix.contract.output }} ${{ matrix.contract.build }}
+      - name: Check Test Data
+        working-directory: ${{ matrix.contract.workdir }}
+        if: ${{ matrix.contract.output != null }}
+        run: >
+          diff ${{ matrix.contract.output }} ${{ matrix.contract.build }}
   
 
   lints:
@@ -107,7 +107,7 @@ jobs:
         uses: actions-rs/toolchain@v1
         with:
           profile: minimal
-          toolchain: nightly
+          toolchain: stable
           override: true
           components: rustfmt, clippy
 

.gitignore

@@ -230,3 +230,7 @@ Cargo.lock
 .beaker
 blocks.db
 **/blocks.db*
+
+# Ignore e2e test artifacts (which clould leak information if commited)
+.ash_history
+.bash_history

app/apptesting/events.go

@@ -21,11 +21,17 @@ func (s *KeeperTestHelper) AssertEventEmitted(ctx sdk.Context, eventTypeExpected
 
 func (s *KeeperTestHelper) FindEvent(events []sdk.Event, name string) sdk.Event {
 	index := slices.IndexFunc(events, func(e sdk.Event) bool { return e.Type == name })
+	if index == -1 {
+		return sdk.Event{}
+	}
 	return events[index]
 }
 
 func (s *KeeperTestHelper) ExtractAttributes(event sdk.Event) map[string]string {
 	attrs := make(map[string]string)
+	if event.Attributes == nil {
+		return attrs
+	}
 	for _, a := range event.Attributes {
 		attrs[string(a.Key)] = string(a.Value)
 	}

app/keepers/keepers.go

@@ -2,6 +2,7 @@ package keepers
 
 import (
 	"github.com/CosmWasm/wasmd/x/wasm"
+	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -32,6 +33,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/upgrade"
 	upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
+	ibcratelimit "github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit"
+	ibcratelimittypes "github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit/types"
 
 	icahost "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/host"
 	icahostkeeper "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/host/keeper"
@@ -110,10 +113,13 @@ type AppKeepers struct {
 	SuperfluidKeeper     *superfluidkeeper.Keeper
 	GovKeeper            *govkeeper.Keeper
 	WasmKeeper           *wasm.Keeper
+	ContractKeeper       *wasmkeeper.PermissionedKeeper
 	TokenFactoryKeeper   *tokenfactorykeeper.Keeper
+
 	// IBC modules
 	// transfer module
-	TransferModule transfer.AppModule
+	TransferModule          transfer.AppModule
+	RateLimitingICS4Wrapper *ibcratelimit.ICS4Wrapper
 
 	// keys to access the substores
 	keys    map[string]*sdk.KVStoreKey
@@ -195,12 +201,24 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 		appKeepers.ScopedIBCKeeper,
 	)
 
+	// 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.IBCKeeper.ChannelKeeper,
+		appKeepers.AccountKeeper,
+		nil,
+		appKeepers.BankKeeper,
+		rateLimitingParams,
+	)
+	appKeepers.RateLimitingICS4Wrapper = &rateLimitingICS4Wrapper
+
 	// Create Transfer Keepers
 	transferKeeper := ibctransferkeeper.NewKeeper(
 		appCodec,
 		appKeepers.keys[ibctransfertypes.StoreKey],
 		appKeepers.GetSubspace(ibctransfertypes.ModuleName),
-		appKeepers.IBCKeeper.ChannelKeeper,
+		appKeepers.RateLimitingICS4Wrapper, // The ICS4Wrapper is replaced by the rateLimitingICS4Wrapper instead of the channel
 		appKeepers.IBCKeeper.ChannelKeeper,
 		&appKeepers.IBCKeeper.PortKeeper,
 		appKeepers.AccountKeeper,
@@ -211,6 +229,9 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 	appKeepers.TransferModule = transfer.NewAppModule(*appKeepers.TransferKeeper)
 	transferIBCModule := transfer.NewIBCModule(*appKeepers.TransferKeeper)
 
+	// RateLimiting IBC Middleware
+	rateLimitingTransferModule := ibcratelimit.NewIBCModule(transferIBCModule, appKeepers.RateLimitingICS4Wrapper)
+
 	icaHostKeeper := icahostkeeper.NewKeeper(
 		appCodec, appKeepers.keys[icahosttypes.StoreKey],
 		appKeepers.GetSubspace(icahosttypes.SubModuleName),
@@ -226,7 +247,8 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 	// Create static IBC router, add transfer route, then set and seal it
 	ibcRouter := porttypes.NewRouter()
 	ibcRouter.AddRoute(icahosttypes.SubModuleName, icaHostIBCModule).
-		AddRoute(ibctransfertypes.ModuleName, transferIBCModule)
+		// The transferIBC module is replaced by rateLimitingTransferModule
+		AddRoute(ibctransfertypes.ModuleName, &rateLimitingTransferModule)
 	// Note: the sealing is done after creating wasmd and wiring that up
 
 	// create evidence keeper with router
@@ -343,6 +365,9 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 		wasmOpts...,
 	)
 	appKeepers.WasmKeeper = &wasmKeeper
+	// Update the ICS4Wrapper with the proper contractKeeper
+	appKeepers.ContractKeeper = wasmkeeper.NewDefaultPermissionKeeper(appKeepers.WasmKeeper)
+	appKeepers.RateLimitingICS4Wrapper.ContractKeeper = appKeepers.ContractKeeper
 
 	// wire up x/wasm to IBC
 	ibcRouter.AddRoute(wasm.ModuleName, wasm.NewIBCHandler(appKeepers.WasmKeeper, appKeepers.IBCKeeper.ChannelKeeper))
@@ -437,6 +462,7 @@ func (appKeepers *AppKeepers) initParamsKeeper(appCodec codec.BinaryCodec, legac
 	paramsKeeper.Subspace(wasm.ModuleName)
 	paramsKeeper.Subspace(tokenfactorytypes.ModuleName)
 	paramsKeeper.Subspace(twaptypes.ModuleName)
+	paramsKeeper.Subspace(ibcratelimittypes.ModuleName)
 
 	return paramsKeeper
 }

tests/e2e/configurer/chain/commands.go

@@ -105,7 +105,7 @@ func (n *NodeConfig) FailIBCTransfer(from, recipient, amount string) {
 
 	cmd := []string{"osmosisd", "tx", "ibc-transfer", "transfer", "transfer", "channel-0", recipient, amount, fmt.Sprintf("--from=%s", from)}
 
-	_, _, err := n.containerManager.ExecTxCmdWithSuccessString(n.t, n.chainId, n.Name, cmd, "rate limit exceeded")
+	_, _, err := n.containerManager.ExecTxCmdWithSuccessString(n.t, n.chainId, n.Name, cmd, "Rate Limit exceeded")
 	require.NoError(n.t, err)
 
 	n.LogActionF("Failed to send IBC transfer (as expected)")

tests/e2e/e2e_test.go

@@ -1,12 +1,17 @@
 package e2e
 
 import (
+	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"strconv"
 	"time"
 
+	paramsutils "github.com/cosmos/cosmos-sdk/x/params/client/utils"
+	ibcratelimittypes "github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit/types"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	coretypes "github.com/tendermint/tendermint/rpc/core/types"
 
@@ -107,6 +112,124 @@ func (s *IntegrationTestSuite) TestSuperfluidVoting() {
 	)
 }
 
+// Copy a file from A to B with io.Copy
+func copyFile(a, b string) error {
+	source, err := os.Open(a)
+	if err != nil {
+		return err
+	}
+	defer source.Close()
+	destination, err := os.Create(b)
+	if err != nil {
+		return err
+	}
+	defer destination.Close()
+	_, err = io.Copy(destination, source)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func (s *IntegrationTestSuite) TestIBCTokenTransferRateLimiting() {
+	if s.skipIBC {
+		s.T().Skip("Skipping IBC tests")
+	}
+	chainA := s.configurer.GetChainConfig(0)
+	chainB := s.configurer.GetChainConfig(1)
+
+	node, err := chainA.GetDefaultNode()
+	s.NoError(err)
+
+	supply, err := node.QueryTotalSupply()
+	s.NoError(err)
+	osmoSupply := supply.AmountOf("uosmo")
+
+	// balance, err := node.QueryBalances(chainA.NodeConfigs[1].PublicAddress)
+	// s.NoError(err)
+
+	f, err := osmoSupply.ToDec().Float64()
+	s.NoError(err)
+
+	over := f * 0.02
+
+	// Sending >1%
+	chainA.SendIBC(chainB, chainB.NodeConfigs[0].PublicAddress, sdk.NewInt64Coin(initialization.OsmoDenom, int64(over)))
+
+	// copy the contract from x/rate-limit/testdata/
+	wd, err := os.Getwd()
+	s.NoError(err)
+	// co up two levels
+	projectDir := filepath.Dir(filepath.Dir(wd))
+	fmt.Println(wd, projectDir)
+	err = copyFile(projectDir+"/x/ibc-rate-limit/testdata/rate_limiter.wasm", wd+"/scripts/rate_limiter.wasm")
+	s.NoError(err)
+	node.StoreWasmCode("rate_limiter.wasm", initialization.ValidatorWalletName)
+	chainA.LatestCodeId += 1
+	node.InstantiateWasmContract(
+		strconv.Itoa(chainA.LatestCodeId),
+		fmt.Sprintf(`{"gov_module": "%s", "ibc_module": "%s", "paths": [{"channel_id": "channel-0", "denom": "%s", "quotas": [{"name":"testQuota", "duration": 86400, "send_recv": [1, 1]}] } ] }`, node.PublicAddress, node.PublicAddress, initialization.OsmoToken.Denom),
+		initialization.ValidatorWalletName)
+
+	// Using code_id 1 because this is the only contract right now. This may need to change if more contracts are added
+	contracts, err := node.QueryContractsFromId(chainA.LatestCodeId)
+	s.NoError(err)
+	s.Require().Len(contracts, 1, "Wrong number of contracts for the rate limiter")
+
+	proposal := paramsutils.ParamChangeProposalJSON{
+		Title:       "Param Change",
+		Description: "Changing the rate limit contract param",
+		Changes: paramsutils.ParamChangesJSON{
+			paramsutils.ParamChangeJSON{
+				Subspace: ibcratelimittypes.ModuleName,
+				Key:      "contract",
+				Value:    []byte(fmt.Sprintf(`"%s"`, contracts[0])),
+			},
+		},
+		Deposit: "625000000uosmo",
+	}
+	proposalJson, err := json.Marshal(proposal)
+	s.NoError(err)
+
+	node.SubmitParamChangeProposal(string(proposalJson), initialization.ValidatorWalletName)
+	chainA.LatestProposalNumber += 1
+
+	for _, n := range chainA.NodeConfigs {
+		n.VoteYesProposal(initialization.ValidatorWalletName, chainA.LatestProposalNumber)
+	}
+
+	// The value is returned as a string, so we have to unmarshal twice
+	type Params struct {
+		Key      string `json:"key"`
+		Subspace string `json:"subspace"`
+		Value    string `json:"value"`
+	}
+
+	s.Eventually(
+		func() bool {
+			var params Params
+			node.QueryParams(ibcratelimittypes.ModuleName, "contract", &params)
+			var val string
+			err := json.Unmarshal([]byte(params.Value), &val)
+			if err != nil {
+				return false
+			}
+			return val != ""
+		},
+		1*time.Minute,
+		10*time.Millisecond,
+		"Osmosis node failed to retrieve params",
+	)
+
+	// Sending <1%. Should work
+	chainA.SendIBC(chainB, chainB.NodeConfigs[0].PublicAddress, sdk.NewInt64Coin(initialization.OsmoDenom, 1))
+	// Sending >1%. Should fail
+	node.FailIBCTransfer(initialization.ValidatorWalletName, chainB.NodeConfigs[0].PublicAddress, fmt.Sprintf("%duosmo", int(over)))
+
+	// Removing the rate limit so it doesn't affect other tests
+	node.WasmExecute(contracts[0], `{"remove_path": {"channel_id": "channel-0", "denom": "uosmo"}}`, initialization.ValidatorWalletName)
+}
+
 // TestAddToExistingLockPostUpgrade ensures addToExistingLock works for locks created preupgrade.
 func (s *IntegrationTestSuite) TestAddToExistingLockPostUpgrade() {
 	if s.skipUpgrade {

x/ibc-rate-limit/contracts/rate-limiter/src/contract_tests.rs

@@ -52,7 +52,7 @@ fn consume_allowance() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_300_u32.into(),
         funds: 300_u32.into(),
     };
     let res = sudo(deps.as_mut(), mock_env(), msg).unwrap();
@@ -64,7 +64,7 @@ fn consume_allowance() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_300_u32.into(),
         funds: 300_u32.into(),
     };
     let err = sudo(deps.as_mut(), mock_env(), msg).unwrap_err();
@@ -91,7 +91,7 @@ fn symetric_flows_dont_consume_allowance() {
     let send_msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_300_u32.into(),
         funds: 300_u32.into(),
     };
     let recv_msg = SudoMsg::RecvPacket {
@@ -154,7 +154,7 @@ fn asymetric_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_060_u32.into(),
         funds: 60_u32.into(),
     };
     let res = sudo(deps.as_mut(), mock_env(), msg).unwrap();
@@ -166,7 +166,7 @@ fn asymetric_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_060_u32.into(),
         funds: 60_u32.into(),
     };
 
@@ -195,7 +195,7 @@ fn asymetric_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_060_u32.into(),
         funds: 60_u32.into(),
     };
     let err = sudo(deps.as_mut(), mock_env(), msg.clone()).unwrap_err();
@@ -205,7 +205,7 @@ fn asymetric_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_060_u32.into(),
         funds: 30_u32.into(),
     };
     let res = sudo(deps.as_mut(), mock_env(), msg.clone()).unwrap();
@@ -256,7 +256,7 @@ fn query_state() {
     let send_msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_300_u32.into(),
         funds: 300_u32.into(),
     };
     sudo(deps.as_mut(), mock_env(), send_msg.clone()).unwrap();
@@ -343,7 +343,7 @@ fn undo_send() {
     let send_msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 3_000_u32.into(),
+        channel_value: 3_300_u32.into(),
         funds: 300_u32.into(),
     };
     let undo_msg = SudoMsg::UndoSend {