IBC Rate Limit - integration and tests

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

.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

@@ -4,12 +4,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"
 
@@ -110,6 +115,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/error.rs

@@ -9,10 +9,14 @@ pub enum ContractError {
     #[error("Unauthorized")]
     Unauthorized {},
 
-    #[error("IBC Rate Limit exceded for channel {channel:?} and denom {denom:?}. Try again after {reset:?}")]
+    #[error("IBC Rate Limit exceeded for {channel}/{denom}. Tried to transfer {amount} which exceeds capacity on the '{quota_name}' quota ({used}/{max}). Try again after {reset:?}")]
     RateLimitExceded {
         channel: String,
         denom: String,
+        amount: u128,
+        quota_name: String,
+        used: u128,
+        max: u128,
         reset: Timestamp,
     },
 

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

@@ -1,3 +1,5 @@
+#![allow(clippy::result_large_err)]
+
 // Contract
 pub mod contract;
 mod error;

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

@@ -102,6 +102,15 @@ impl Flow {
         }
     }
 
+    /// returns the balance in a direction. This is used for displaying cleaner errors
+    pub fn balance_on(&self, direction: &FlowType) -> u128 {
+        let (balance_in, balance_out) = self.balance();
+        match direction {
+            FlowType::In => balance_in,
+            FlowType::Out => balance_out,
+        }
+    }
+
     /// If now is greater than the period_end, the Flow is considered expired.
     pub fn is_expired(&self, now: Timestamp) -> bool {
         self.period_end < now
@@ -182,6 +191,15 @@ impl Quota {
             None => (0_u32.into(), 0_u32.into()), // This should never happen, but ig the channel value is not set, we disallow any transfer
         }
     }
+
+    /// returns the capacity in a direction. This is used for displaying cleaner errors
+    pub fn capacity_on(&self, direction: &FlowType) -> u128 {
+        let (max_in, max_out) = self.capacity();
+        match direction {
+            FlowType::In => max_in,
+            FlowType::Out => max_out,
+        }
+    }
 }
 
 impl From<&QuotaMsg> for Quota {
@@ -224,6 +242,13 @@ impl RateLimit {
         channel_value: Uint256,
         now: Timestamp,
     ) -> Result<Self, ContractError> {
+        // Flow used before this transaction is applied.
+        // This is used to make error messages more informative
+        let initial_flow = self.flow.balance_on(direction);
+
+        // Apply the transfer. From here on, we will updated the flow with the new transfer
+        // and check if  it exceeds the quota at the current time
+
         let expired = self.flow.apply_transfer(direction, funds, now, &self.quota);
         // Cache the channel value if it has never been set or it has expired.
         if self.quota.channel_value.is_none() || expired {
@@ -236,6 +261,10 @@ impl RateLimit {
             true => Err(ContractError::RateLimitExceded {
                 channel: path.channel.to_string(),
                 denom: path.denom.to_string(),
+                amount: funds,
+                quota_name: self.quota.name.to_string(),
+                used: initial_flow,
+                max: self.quota.capacity_on(direction),
                 reset: self.flow.period_end,
             }),
             false => Ok(RateLimit {