Rate limit - Cleaner tests (#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 9ffdc37c.

* 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 expe...

.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
\ No newline at end of file

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

 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 {

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

-use cosmwasm_std::{StdError, Timestamp};
+use cosmwasm_std::{StdError, Timestamp, Uint256};
 use thiserror::Error;
 
 #[derive(Error, Debug)]
@@ -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: Uint256,
+        quota_name: String,
+        used: Uint256,
+        max: Uint256,
         reset: Timestamp,
     },
 

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

@@ -82,7 +82,7 @@ fn expiration() {
     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 cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -105,7 +105,7 @@ fn expiration() {
     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 cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -123,7 +123,7 @@ fn expiration() {
     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(),
     };
 
@@ -162,7 +162,7 @@ fn multiple_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 100_u32.into(),
+        channel_value: 101_u32.into(),
         funds: 1_u32.into(),
     };
     let cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -172,7 +172,7 @@ fn multiple_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 100_u32.into(),
+        channel_value: 101_u32.into(),
         funds: 1_u32.into(),
     };
     let cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -188,7 +188,7 @@ fn multiple_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 100_u32.into(),
+        channel_value: 101_u32.into(),
         funds: 1_u32.into(),
     };
 
@@ -207,7 +207,7 @@ fn multiple_quotas() {
         let msg = SudoMsg::SendPacket {
             channel_id: format!("channel"),
             denom: format!("denom"),
-            channel_value: 100_u32.into(),
+            channel_value: 101_u32.into(),
             funds: 1_u32.into(),
         };
         let cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -224,7 +224,7 @@ fn multiple_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 100_u32.into(),
+        channel_value: 101_u32.into(),
         funds: 1_u32.into(),
     };
     let cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -240,7 +240,7 @@ fn multiple_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 100_u32.into(),
+        channel_value: 101_u32.into(),
         funds: 1_u32.into(),
     };
     let cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -257,7 +257,7 @@ fn multiple_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 100_u32.into(),
+        channel_value: 101_u32.into(),
         funds: 1_u32.into(),
     };
     let cosmos_msg = cw_rate_limit_contract.sudo(msg);
@@ -272,7 +272,7 @@ fn multiple_quotas() {
     let msg = SudoMsg::SendPacket {
         channel_id: format!("channel"),
         denom: format!("denom"),
-        channel_value: 100_u32.into(),
+        channel_value: 101_u32.into(),
         funds: 1_u32.into(),
     };
     let cosmos_msg = cw_rate_limit_contract.sudo(msg);

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

+use cosmwasm_std::{Addr, Deps, Timestamp};
+use serde::{Deserialize, Serialize};
+
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
+pub struct Height {
+    /// Previously known as "epoch"
+    revision_number: Option<u64>,
+
+    /// The height of a block
+    revision_height: Option<u64>,
+}
+
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
+pub struct FungibleTokenData {
+    denom: String,
+    amount: u128,
+    sender: Addr,
+    receiver: Addr,
+}
+
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
+pub struct Packet {
+    pub sequence: u64,
+    pub source_port: String,
+    pub source_channel: String,
+    pub destination_port: String,
+    pub destination_channel: String,
+    pub data: FungibleTokenData,
+    pub timeout_height: Height,
+    pub timeout_timestamp: Option<Timestamp>,
+}
+
+impl Packet {
+    pub fn channel_value(&self, _deps: Deps) -> u128 {
+        // let balance = deps.querier.query_all_balances("address", self.data.denom);
+        // deps.querier.sup
+        return 125000000000011250 * 2;
+    }
+
+    pub fn get_funds(&self) -> u128 {
+        return self.data.amount;
+    }
+
+    fn local_channel(&self) -> String {
+        // Pick the appropriate channel depending on whether this is a send or a recv
+        return self.destination_channel.clone();
+    }
+
+    fn local_demom(&self) -> String {
+        // This should actually convert the denom from the packet to the osmosis denom, but for now, just returning this
+        return self.data.denom.clone();
+    }
+
+    pub fn path_data(&self) -> (String, String) {
+        let denom = self.local_demom();
+        let channel = if denom.starts_with("ibc/") {
+            self.local_channel()
+        } else {
+            "any".to_string() // native tokens are rate limited globally
+        };
+
+        return (channel, denom);
+    }
+}

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) -> Uint256 {
+        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) -> Uint256 {
+        let (max_in, max_out) = self.capacity();
+        match direction {
+            FlowType::In => max_in,
+            FlowType::Out => max_out,
+        }
+    }
 }
 
 impl From<&QuotaMsg> for Quota {
@@ -209,6 +227,29 @@ pub struct RateLimit {
     pub flow: Flow,
 }
 
+// The channel value on send depends on the amount on escrow. The ibc transfer
+// module modifies the escrow amount by "funds" on sends before calling the
+// contract. This function takes that into account so that the channel value
+// that we track matches the channel value at the moment when the ibc
+// transaction started executing
+fn calculate_channel_value(
+    channel_value: Uint256,
+    denom: &str,
+    funds: Uint256,
+    direction: &FlowType,
+) -> Uint256 {
+    match direction {
+        FlowType::Out => {
+            if denom.contains("ibc") {
+                channel_value + funds // Non-Native tokens get removed from the supply on send. Add that amount back
+            } else {
+                channel_value - funds // Native tokens increase escrow amount on send. Remove that amount here
+            }
+        }
+        FlowType::In => channel_value,
+    }
+}
+
 impl RateLimit {
     /// Checks if a transfer is allowed and updates the data structures
     /// accordingly.
@@ -224,10 +265,22 @@ 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 {
-            self.quota.channel_value = Some(channel_value)
+            self.quota.channel_value = Some(calculate_channel_value(
+                channel_value,
+                &path.denom,
+                funds,
+                direction,
+            ))
         }
 
         let (max_in, max_out) = self.quota.capacity();
@@ -236,6 +289,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 {

x/ibc-rate-limit/ibc_middleware_test.go

+package ibc_rate_limit_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	ibc_rate_limit "github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit"
+
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
+	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
+	ibctesting "github.com/cosmos/ibc-go/v3/testing"
+	"github.com/osmosis-labs/osmosis/v12/app"
+	"github.com/osmosis-labs/osmosis/v12/app/apptesting"
+	"github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit/testutil"
+	"github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit/types"
+	"github.com/stretchr/testify/suite"
+)
+
+type MiddlewareTestSuite struct {
+	apptesting.KeeperTestHelper
+
+	coordinator *ibctesting.Coordinator
+
+	// testing chains used for convenience and readability
+	chainA *osmosisibctesting.TestChain
+	chainB *osmosisibctesting.TestChain
+	path   *ibctesting.Path
+}
+
+// Setup
+func TestMiddlewareTestSuite(t *testing.T) {
+	suite.Run(t, new(MiddlewareTestSuite))
+}
+
+func SetupTestingApp() (ibctesting.TestingApp, map[string]json.RawMessage) {
+	osmosisApp := app.Setup(false)
+	return osmosisApp, app.NewDefaultGenesisState()
+}
+
+func NewTransferPath(chainA, chainB *osmosisibctesting.TestChain) *ibctesting.Path {
+	path := ibctesting.NewPath(chainA.TestChain, chainB.TestChain)
+	path.EndpointA.ChannelConfig.PortID = ibctesting.TransferPort
+	path.EndpointB.ChannelConfig.PortID = ibctesting.TransferPort
+	path.EndpointA.ChannelConfig.Version = transfertypes.Version
+	path.EndpointB.ChannelConfig.Version = transfertypes.Version
+	return path
+}
+
+func (suite *MiddlewareTestSuite) SetupTest() {
+	suite.Setup()
+	ibctesting.DefaultTestingAppInit = SetupTestingApp
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
+	suite.chainA = &osmosisibctesting.TestChain{
+		TestChain: suite.coordinator.GetChain(ibctesting.GetChainID(1)),
+	}
+	// Remove epochs to prevent  minting
+	suite.chainA.MoveEpochsToTheFuture()
+	suite.chainB = &osmosisibctesting.TestChain{
+		TestChain: suite.coordinator.GetChain(ibctesting.GetChainID(2)),
+	}
+	suite.path = NewTransferPath(suite.chainA, suite.chainB)
+	suite.coordinator.Setup(suite.path)
+}
+
+// Helpers
+func (suite *MiddlewareTestSuite) MessageFromAToB(denom string, amount sdk.Int) sdk.Msg {
+	coin := sdk.NewCoin(denom, amount)
+	port := suite.path.EndpointA.ChannelConfig.PortID
+	channel := suite.path.EndpointA.ChannelID
+	accountFrom := suite.chainA.SenderAccount.GetAddress().String()
+	accountTo := suite.chainB.SenderAccount.GetAddress().String()
+	timeoutHeight := clienttypes.NewHeight(0, 100)
+	return transfertypes.NewMsgTransfer(
+		port,
+		channel,
+		coin,
+		accountFrom,
+		accountTo,
+		timeoutHeight,
+		0,
+	)
+}
+
+func (suite *MiddlewareTestSuite) MessageFromBToA(denom string, amount sdk.Int) sdk.Msg {
+	coin := sdk.NewCoin(denom, amount)
+	port := suite.path.EndpointB.ChannelConfig.PortID
+	channel := suite.path.EndpointB.ChannelID
+	accountFrom := suite.chainB.SenderAccount.GetAddress().String()
+	accountTo := suite.chainA.SenderAccount.GetAddress().String()
+	timeoutHeight := clienttypes.NewHeight(0, 100)
+	return transfertypes.NewMsgTransfer(
+		port,
+		channel,
+		coin,
+		accountFrom,
+		accountTo,
+		timeoutHeight,
+		0,
+	)
+}
+
+// Tests that a receiver address longer than 4096 is not accepted
+func (suite *MiddlewareTestSuite) TestInvalidReceiver() {
+	msg := transfertypes.NewMsgTransfer(
+		suite.path.EndpointB.ChannelConfig.PortID,
+		suite.path.EndpointB.ChannelID,
+		sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1)),
+		suite.chainB.SenderAccount.GetAddress().String(),
+		strings.Repeat("x", 4097),
+		clienttypes.NewHeight(0, 100),
+		0,
+	)
+	_, ack, _ := suite.FullSendBToA(msg)
+	suite.Require().Contains(string(ack), "error",
+		"acknowledgment is not an error")
+	suite.Require().Contains(string(ack), sdkerrors.ErrInvalidAddress.Error(),
+		"acknowledgment error is not of the right type")
+}
+
+func (suite *MiddlewareTestSuite) FullSendBToA(msg sdk.Msg) (*sdk.Result, string, error) {
+	sendResult, err := suite.chainB.SendMsgsNoCheck(msg)
+	suite.Require().NoError(err)
+
+	packet, err := ibctesting.ParsePacketFromEvents(sendResult.GetEvents())
+	suite.Require().NoError(err)
+
+	err = suite.path.EndpointA.UpdateClient()
+	suite.Require().NoError(err)
+
+	res, err := suite.path.EndpointA.RecvPacketWithResult(packet)
+	suite.Require().NoError(err)
+
+	ack, err := ibctesting.ParseAckFromEvents(res.GetEvents())
+
+	err = suite.path.EndpointA.UpdateClient()
+	suite.Require().NoError(err)
+	err = suite.path.EndpointB.UpdateClient()
+	suite.Require().NoError(err)
+
+	return sendResult, string(ack), err
+}
+
+func (suite *MiddlewareTestSuite) FullSendAToB(msg sdk.Msg) (*sdk.Result, string, error) {
+	sendResult, err := suite.chainA.SendMsgsNoCheck(msg)
+	if err != nil {
+		return nil, "", err
+	}
+
+	packet, err := ibctesting.ParsePacketFromEvents(sendResult.GetEvents())
+	if err != nil {
+		return nil, "", err
+	}
+
+	err = suite.path.EndpointB.UpdateClient()
+	if err != nil {
+		return nil, "", err
+	}
+
+	res, err := suite.path.EndpointB.RecvPacketWithResult(packet)
+	if err != nil {
+		return nil, "", err
+	}
+
+	ack, err := ibctesting.ParseAckFromEvents(res.GetEvents())
+	if err != nil {
+		return nil, "", err
+	}
+
+	err = suite.path.EndpointA.UpdateClient()
+	if err != nil {
+		return nil, "", err
+	}
+	err = suite.path.EndpointB.UpdateClient()
+	if err != nil {
+		return nil, "", err
+	}
+
+	return sendResult, string(ack), nil
+}
+
+func (suite *MiddlewareTestSuite) AssertReceive(success bool, msg sdk.Msg) (string, error) {
+	_, ack, err := suite.FullSendBToA(msg)
+	if success {
+		suite.Require().NoError(err)
+		suite.Require().NotContains(string(ack), "error",
+			"acknowledgment is an error")
+	} else {
+		suite.Require().Contains(string(ack), "error",
+			"acknowledgment is not an error")
+		suite.Require().Contains(string(ack), types.ErrRateLimitExceeded.Error(),
+			"acknowledgment error is not of the right type")
+	}
+	return ack, err
+}
+
+func (suite *MiddlewareTestSuite) AssertSend(success bool, msg sdk.Msg) (*sdk.Result, error) {
+	r, _, err := suite.FullSendAToB(msg)
+	if success {
+		suite.Require().NoError(err, "IBC send failed. Expected success. %s", err)
+	} else {
+		suite.Require().Error(err, "IBC send succeeded. Expected failure")
+		suite.ErrorContains(err, types.ErrRateLimitExceeded.Error(), "Bad error type")
+	}
+	return r, err
+}
+
+func (suite *MiddlewareTestSuite) BuildChannelQuota(name, denom string, duration, send_precentage, recv_percentage uint32) string {
+	return fmt.Sprintf(`
+          {"channel_id": "channel-0", "denom": "%s", "quotas": [{"name":"%s", "duration": %d, "send_recv":[%d, %d]}] }
+    `, denom, name, duration, send_precentage, recv_percentage)
+}
+
+// Tests
+
+// Test that Sending IBC messages works when the middleware isn't configured
+func (suite *MiddlewareTestSuite) TestSendTransferNoContract() {
+	one := sdk.NewInt(1)
+	suite.AssertSend(true, suite.MessageFromAToB(sdk.DefaultBondDenom, one))
+}
+
+// Test that Receiving IBC messages works when the middleware isn't configured
+func (suite *MiddlewareTestSuite) TestReceiveTransferNoContract() {
+	one := sdk.NewInt(1)
+	suite.AssertReceive(true, suite.MessageFromBToA(sdk.DefaultBondDenom, one))
+}
+
+func (suite *MiddlewareTestSuite) initializeEscrow() (totalEscrow, expectedSed sdk.Int) {
+	osmosisApp := suite.chainA.GetOsmosisApp()
+	supply := osmosisApp.BankKeeper.GetSupplyWithOffset(suite.chainA.GetContext(), sdk.DefaultBondDenom)
+
+	// Move some funds from chainA to chainB so that there is something in escrow
+	// Each user has 10% of the supply, so we send most of the funds from one user to chainA
+	transferAmount := supply.Amount.QuoRaw(20)
+
+	// When sending, the amount we're sending goes into escrow before we enter the middleware and thus
+	// it's used as part of the channel value in the rate limiting contract
+	// To account for that, we subtract the amount we'll send first (2.5% of transferAmount) here
+	sendAmount := transferAmount.QuoRaw(40)
+
+	// Send from A to B
+	_, _, err := suite.FullSendAToB(suite.MessageFromAToB(sdk.DefaultBondDenom, transferAmount.Sub(sendAmount)))
+	suite.Require().NoError(err)
+	// Send from A to B
+	_, _, err = suite.FullSendBToA(suite.MessageFromBToA(sdk.DefaultBondDenom, transferAmount.Sub(sendAmount)))
+	suite.Require().NoError(err)
+
+	return transferAmount, sendAmount
+}
+
+func (suite *MiddlewareTestSuite) fullSendTest(native bool) map[string]string {
+	quotaPercentage := 5
+	suite.initializeEscrow()
+	// Get the denom and amount to send
+	denom := sdk.DefaultBondDenom
+	if !native {
+		denomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom("transfer", "channel-0", denom))
+		denom = denomTrace.IBCDenom()
+	}
+
+	osmosisApp := suite.chainA.GetOsmosisApp()
+
+	// This is the first one. Inside the tests. It works as expected.
+	channelValue := ibc_rate_limit.CalculateChannelValue(suite.chainA.GetContext(), denom, "transfer", "channel-0", osmosisApp.BankKeeper)
+
+	// The amount to be sent is send 2.5% (quota is 5%)
+	quota := channelValue.QuoRaw(int64(100 / quotaPercentage))
+	sendAmount := quota.QuoRaw(2)
+
+	fmt.Printf("Testing send rate limiting for denom=%s, channelValue=%s, quota=%s, sendAmount=%s\n", denom, channelValue, quota, sendAmount)
+
+	// Setup contract
+	suite.chainA.StoreContractCode(&suite.Suite)
+	quotas := suite.BuildChannelQuota("weekly", denom, 604800, 5, 5)
+	fmt.Println(quotas)
+	addr := suite.chainA.InstantiateContract(&suite.Suite, quotas)
+	suite.chainA.RegisterRateLimitingContract(addr)
+
+	// TODO: Remove native from MessafeFrom calls
+	// send 2.5% (quota is 5%)
+	fmt.Println("trying to send ", sendAmount)
+	suite.AssertSend(true, suite.MessageFromAToB(denom, sendAmount))
+
+	// send 2.5% (quota is 5%)
+	fmt.Println("trying to send ", sendAmount)
+	r, _ := suite.AssertSend(true, suite.MessageFromAToB(denom, sendAmount))
+
+	// Calculate remaining allowance in the quota
+	attrs := suite.ExtractAttributes(suite.FindEvent(r.GetEvents(), "wasm"))
+
+	used, ok := sdk.NewIntFromString(attrs["weekly_used_out"])
+	suite.Require().True(ok)
+
+	suite.Require().Equal(used, sendAmount.MulRaw(2))
+
+	// Sending above the quota should fail. We use 2 instead of 1 here to avoid rounding issues
+	suite.AssertSend(false, suite.MessageFromAToB(denom, sdk.NewInt(2)))
+	return attrs
+}
+
+// Test rate limiting on sends
+func (suite *MiddlewareTestSuite) TestSendTransferWithRateLimitingNative() {
+	suite.fullSendTest(true)
+}
+
+// Test rate limiting on sends
+func (suite *MiddlewareTestSuite) TestSendTransferWithRateLimitingNonNative() {
+	suite.fullSendTest(false)
+}
+
+// Test rate limits are reset when the specified time period has passed
+func (suite *MiddlewareTestSuite) TestSendTransferReset() {
+	// Same test as above, but the quotas get reset after time passes
+	attrs := suite.fullSendTest(true)
+	parts := strings.Split(attrs["weekly_period_end"], ".") // Splitting timestamp into secs and nanos
+	secs, err := strconv.ParseInt(parts[0], 10, 64)
+	suite.Require().NoError(err)
+	nanos, err := strconv.ParseInt(parts[1], 10, 64)
+	suite.Require().NoError(err)
+	resetTime := time.Unix(secs, nanos)
+
+	// Move chainA forward one block
+	suite.chainA.NextBlock()
+	suite.chainA.SenderAccount.SetSequence(suite.chainA.SenderAccount.GetSequence() + 1)
+
+	// Reset time + one second
+	oneSecAfterReset := resetTime.Add(time.Second)
+	suite.coordinator.IncrementTimeBy(oneSecAfterReset.Sub(suite.coordinator.CurrentTime))
+
+	// Sending should succeed again
+	suite.AssertSend(true, suite.MessageFromAToB(sdk.DefaultBondDenom, sdk.NewInt(1)))
+}
+
+// Test rate limiting on receives
+func (suite *MiddlewareTestSuite) fullRecvTest(native bool) {
+	quotaPercentage := 5
+	suite.initializeEscrow()
+	// Get the denom and amount to send
+	denom := sdk.DefaultBondDenom
+	if !native {
+		denomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom("transfer", "channel-0", denom))
+		denom = denomTrace.IBCDenom()
+	}
+
+	osmosisApp := suite.chainA.GetOsmosisApp()
+
+	// This is the first one. Inside the tests. It works as expected.
+	channelValue := ibc_rate_limit.CalculateChannelValue(suite.chainA.GetContext(), denom, "transfer", "channel-0", osmosisApp.BankKeeper)
+
+	// The amount to be sent is send 2.5% (quota is 5%)
+	quota := channelValue.QuoRaw(int64(100 / quotaPercentage))
+	sendAmount := quota.QuoRaw(2)
+
+	fmt.Printf("Testing recv rate limiting for denom=%s, channelValue=%s, quota=%s, sendAmount=%s\n", denom, channelValue, quota, sendAmount)
+
+	// Setup contract
+	suite.chainA.StoreContractCode(&suite.Suite)
+	quotas := suite.BuildChannelQuota("weekly", denom, 604800, 5, 5)
+	addr := suite.chainA.InstantiateContract(&suite.Suite, quotas)
+	suite.chainA.RegisterRateLimitingContract(addr)
+
+	// receive 2.5% (quota is 5%)
+	suite.AssertReceive(true, suite.MessageFromBToA(denom, sendAmount))
+
+	// receive 2.5% (quota is 5%)
+	suite.AssertReceive(true, suite.MessageFromBToA(denom, sendAmount))
+
+	// Sending above the quota should fail. We send 2 instead of 1 to account for rounding errors
+	suite.AssertReceive(false, suite.MessageFromBToA(denom, sdk.NewInt(2)))
+}
+
+func (suite *MiddlewareTestSuite) TestRecvTransferWithRateLimitingNative() {
+	suite.fullRecvTest(true)
+}
+
+func (suite *MiddlewareTestSuite) TestRecvTransferWithRateLimitingNonNative() {
+	suite.fullRecvTest(false)
+}
+
+// Test no rate limiting occurs when the contract is set, but not quotas are condifured for the path
+func (suite *MiddlewareTestSuite) TestSendTransferNoQuota() {
+	// Setup contract
+	suite.chainA.StoreContractCode(&suite.Suite)
+	addr := suite.chainA.InstantiateContract(&suite.Suite, ``)
+	suite.chainA.RegisterRateLimitingContract(addr)
+
+	// send 1 token.
+	// If the contract doesn't have a quota for the current channel, all transfers are allowed
+	suite.AssertSend(true, suite.MessageFromAToB(sdk.DefaultBondDenom, sdk.NewInt(1)))
+}
+
+// Test rate limits are reverted if a "send" fails
+func (suite *MiddlewareTestSuite) TestFailedSendTransfer() {
+	suite.initializeEscrow()
+	// Setup contract
+	suite.chainA.StoreContractCode(&suite.Suite)
+	quotas := suite.BuildChannelQuota("weekly", sdk.DefaultBondDenom, 604800, 1, 1)
+	addr := suite.chainA.InstantiateContract(&suite.Suite, quotas)
+	suite.chainA.RegisterRateLimitingContract(addr)
+
+	// Get the escrowed amount
+	osmosisApp := suite.chainA.GetOsmosisApp()
+	escrowAddress := transfertypes.GetEscrowAddress("transfer", "channel-0")
+	escrowed := osmosisApp.BankKeeper.GetBalance(suite.chainA.GetContext(), escrowAddress, sdk.DefaultBondDenom)
+
+	quota := escrowed.Amount.QuoRaw(100) // 1% of the escrowed amount
+
+	// Use the whole quota
+	coins := sdk.NewCoin(sdk.DefaultBondDenom, quota)
+	port := suite.path.EndpointA.ChannelConfig.PortID
+	channel := suite.path.EndpointA.ChannelID
+	accountFrom := suite.chainA.SenderAccount.GetAddress().String()
+	timeoutHeight := clienttypes.NewHeight(0, 100)
+	msg := transfertypes.NewMsgTransfer(port, channel, coins, accountFrom, "INVALID", timeoutHeight, 0)
+
+	// Sending the message manually because AssertSend updates both clients. We need to update the clients manually
+	// for this test so that the failure to receive on chain B happens after the second packet is sent from chain A.
+	// That way we validate that chain A is blocking as expected, but the flow is reverted after the receive failure is
+	// acknowledged on chain A
+	res, err := suite.chainA.SendMsgsNoCheck(msg)
+	suite.Require().NoError(err)
+
+	// Sending again fails as the quota is filled
+	suite.AssertSend(false, suite.MessageFromAToB(sdk.DefaultBondDenom, quota))
+
+	// Move forward one block
+	suite.chainA.NextBlock()
+	suite.chainA.SenderAccount.SetSequence(suite.chainA.SenderAccount.GetSequence() + 1)
+	suite.chainA.Coordinator.IncrementTime()
+
+	// Update both clients
+	err = suite.path.EndpointA.UpdateClient()
+	suite.Require().NoError(err)
+	err = suite.path.EndpointB.UpdateClient()
+	suite.Require().NoError(err)
+
+	// Execute the acknowledgement from chain B in chain A
+
+	// extract the sent packet
+	packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents())
+	suite.Require().NoError(err)
+
+	// recv in chain b
+	res, err = suite.path.EndpointB.RecvPacketWithResult(packet)
+
+	// get the ack from the chain b's response
+	ack, err := ibctesting.ParseAckFromEvents(res.GetEvents())
+	suite.Require().NoError(err)
+
+	// manually relay it to chain a
+	err = suite.path.EndpointA.AcknowledgePacket(packet, ack)
+	suite.Require().NoError(err)
+
+	// We should be able to send again because the packet that exceeded the quota failed and has been reverted
+	suite.AssertSend(true, suite.MessageFromAToB(sdk.DefaultBondDenom, sdk.NewInt(1)))
+}

x/ibc-rate-limit/ibc_module.go

@@ -103,12 +103,27 @@ func (im *IBCModule) OnChanCloseConfirm(
 	return im.app.OnChanCloseConfirm(ctx, portID, channelID)
 }
 
+func ValidateReceiverAddress(packet channeltypes.Packet) error {
+	var packetData transfertypes.FungibleTokenPacketData
+	if err := json.Unmarshal(packet.GetData(), &packetData); err != nil {
+		return err
+	}
+	if len(packetData.Receiver) >= 4096 {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "IBC Receiver address too long. Max supported length is %d", 4096)
+	}
+	return nil
+}
+
 // OnRecvPacket implements the IBCModule interface
 func (im *IBCModule) OnRecvPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	relayer sdk.AccAddress,
 ) exported.Acknowledgement {
+	if err := ValidateReceiverAddress(packet); err != nil {
+		return channeltypes.NewErrorAcknowledgement(err.Error())
+	}
+
 	contract := im.ics4Middleware.GetParams(ctx)
 	if contract == "" {
 		// The contract has not been configured. Continue as usual
@@ -116,9 +131,10 @@ func (im *IBCModule) OnRecvPacket(
 	}
 	amount, denom, err := GetFundsFromPacket(packet)
 	if err != nil {
-		return channeltypes.NewErrorAcknowledgement("bad packet")
+		return channeltypes.NewErrorAcknowledgement("bad packet in rate limit's OnRecvPacket")
 	}
-	channelValue := im.ics4Middleware.CalculateChannelValue(ctx, denom)
+
+	channelValue := im.ics4Middleware.CalculateChannelValue(ctx, denom, packet)
 
 	err = CheckAndUpdateRateLimits(
 		ctx,
@@ -127,11 +143,11 @@ func (im *IBCModule) OnRecvPacket(
 		contract,
 		channelValue,
 		packet.GetDestChannel(),
-		denom,
+		denom, // We always use the packet's denom here, as we want the limits to be the same on both directions
 		amount,
 	)
 	if err != nil {
-		return channeltypes.NewErrorAcknowledgement(types.RateLimitExceededMsg)
+		return channeltypes.NewErrorAcknowledgement(types.ErrRateLimitExceeded.Error())
 	}
 
 	// if this returns an Acknowledgement that isn't successful, all state changes are discarded

x/ibc-rate-limit/ics4_wrapper.go

@@ -53,9 +53,11 @@ func (i *ICS4Wrapper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capab
 
 	amount, denom, err := GetFundsFromPacket(packet)
 	if err != nil {
-		return sdkerrors.Wrap(err, "Rate limited SendPacket")
+		return sdkerrors.Wrap(err, "Rate limit SendPacket")
 	}
-	channelValue := i.CalculateChannelValue(ctx, denom)
+
+	channelValue := i.CalculateChannelValue(ctx, denom, packet)
+
 	err = CheckAndUpdateRateLimits(
 		ctx,
 		i.ContractKeeper,
@@ -63,11 +65,11 @@ func (i *ICS4Wrapper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capab
 		contract,
 		channelValue,
 		packet.GetSourceChannel(),
-		denom,
+		denom, // We always use the packet's denom here, as we want the limits to be the same on both directions
 		amount,
 	)
 	if err != nil {
-		return sdkerrors.Wrap(err, "Rate limited SendPacket")
+		return sdkerrors.Wrap(err, "bad packet in rate limit's SendPacket")
 	}
 
 	return i.channel.SendPacket(ctx, chanCap, packet)
@@ -84,6 +86,7 @@ func (i *ICS4Wrapper) GetParams(ctx sdk.Context) (contract string) {
 
 // CalculateChannelValue The value of an IBC channel. This is calculated using the denom supplied by the sender.
 // if the denom is not correct, the transfer should fail somewhere else on the call chain
-func (i *ICS4Wrapper) CalculateChannelValue(ctx sdk.Context, denom string) sdk.Int {
-	return i.bankKeeper.GetSupplyWithOffset(ctx, denom).Amount
+func (i *ICS4Wrapper) CalculateChannelValue(ctx sdk.Context, denom string, packet exported.PacketI) sdk.Int {
+	// The logic is etracted into a function here so that it can be used within the tests
+	return CalculateChannelValue(ctx, denom, packet.GetSourcePort(), packet.GetSourceChannel(), i.bankKeeper)
 }

x/ibc-rate-limit/rate_limit.go

@@ -2,10 +2,13 @@ package ibc_rate_limit
 
 import (
 	"encoding/json"
+	"strings"
 
 	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
+	transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
 	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 	"github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit/types"
 )
@@ -15,11 +18,6 @@ var (
 	msgRecv = "recv_packet"
 )
 
-type PacketData struct {
-	Denom  string `json:"denom"`
-	Amount string `json:"amount"`
-}
-
 func CheckAndUpdateRateLimits(ctx sdk.Context, contractKeeper *wasmkeeper.PermissionedKeeper,
 	msgType, contract string,
 	channelValue sdk.Int, sourceChannel, denom string,
@@ -42,6 +40,7 @@ func CheckAndUpdateRateLimits(ctx sdk.Context, contractKeeper *wasmkeeper.Permis
 	}
 
 	_, err = contractKeeper.Sudo(ctx, contractAddr, sendPacketMsg)
+
 	if err != nil {
 		return sdkerrors.Wrap(types.ErrRateLimitExceeded, err.Error())
 	}
@@ -128,10 +127,41 @@ func BuildWasmExecMsg(msgType, sourceChannel, denom string, channelValue sdk.Int
 }
 
 func GetFundsFromPacket(packet exported.PacketI) (string, string, error) {
-	var packetData PacketData
+	var packetData transfertypes.FungibleTokenPacketData
 	err := json.Unmarshal(packet.GetData(), &packetData)
 	if err != nil {
 		return "", "", err
 	}
-	return packetData.Amount, packetData.Denom, nil
+	return packetData.Amount, GetLocalDenom(packetData.Denom), nil
+}
+
+func GetLocalDenom(denom string) string {
+	// Expected denoms in the following cases:
+	//
+	// send non-native: transfer/channel-0/denom -> ibc/xxx
+	// send native: denom -> denom
+	// recv (B)non-native: denom
+	// recv (B)native: transfer/channel-0/denom
+	//
+	if strings.HasPrefix(denom, "transfer/") {
+		denomTrace := transfertypes.ParseDenomTrace(denom)
+		return denomTrace.IBCDenom()
+	} else {
+		return denom
+	}
+}
+
+func CalculateChannelValue(ctx sdk.Context, denom string, port, channel string, bankKeeper bankkeeper.Keeper) sdk.Int {
+	if strings.HasPrefix(denom, "ibc/") {
+		return bankKeeper.GetSupplyWithOffset(ctx, denom).Amount
+	}
+
+	if channel == "any" {
+		// ToDo: Get all channels and sum the escrow addr value over all the channels
+		escrowAddress := transfertypes.GetEscrowAddress(port, channel)
+		return bankKeeper.GetBalance(ctx, escrowAddress, denom).Amount
+	} else {
+		escrowAddress := transfertypes.GetEscrowAddress(port, channel)
+		return bankKeeper.GetBalance(ctx, escrowAddress, denom).Amount
+	}
 }

x/ibc-rate-limit/testutil/chain.go

+package osmosisibctesting
+
+import (
+	"time"
+
+	"github.com/cosmos/cosmos-sdk/baseapp"
+	"github.com/cosmos/cosmos-sdk/client"
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	ibctesting "github.com/cosmos/ibc-go/v3/testing"
+	"github.com/cosmos/ibc-go/v3/testing/simapp/helpers"
+	"github.com/osmosis-labs/osmosis/v12/app"
+	abci "github.com/tendermint/tendermint/abci/types"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+)
+
+type TestChain struct {
+	*ibctesting.TestChain
+}
+
+// SendMsgsNoCheck overrides ibctesting.TestChain.SendMsgs so that it doesn't check for errors. That should be handled by the caller
+func (chain *TestChain) SendMsgsNoCheck(msgs ...sdk.Msg) (*sdk.Result, error) {
+	// ensure the chain has the latest time
+	chain.Coordinator.UpdateTimeForChain(chain.TestChain)
+
+	_, r, err := SignAndDeliver(
+		chain.TxConfig,
+		chain.App.GetBaseApp(),
+		chain.GetContext().BlockHeader(),
+		msgs,
+		chain.ChainID,
+		[]uint64{chain.SenderAccount.GetAccountNumber()},
+		[]uint64{chain.SenderAccount.GetSequence()},
+		chain.SenderPrivKey,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	// SignAndDeliver calls app.Commit()
+	chain.NextBlock()
+
+	// increment sequence for successful transaction execution
+	err = chain.SenderAccount.SetSequence(chain.SenderAccount.GetSequence() + 1)
+	if err != nil {
+		return nil, err
+	}
+
+	chain.Coordinator.IncrementTime()
+
+	return r, nil
+}
+
+// SignAndDeliver signs and delivers a transaction without asserting the results. This overrides the function
+// from ibctesting
+func SignAndDeliver(
+	txCfg client.TxConfig, app *baseapp.BaseApp, header tmproto.Header, msgs []sdk.Msg,
+	chainID string, accNums, accSeqs []uint64, priv ...cryptotypes.PrivKey,
+) (sdk.GasInfo, *sdk.Result, error) {
+	tx, _ := helpers.GenTx(
+		txCfg,
+		msgs,
+		sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 0)},
+		helpers.DefaultGenTxGas,
+		chainID,
+		accNums,
+		accSeqs,
+		priv...,
+	)
+
+	// Simulate a sending a transaction and committing a block
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
+	gInfo, res, err := app.Deliver(txCfg.TxEncoder(), tx)
+
+	app.EndBlock(abci.RequestEndBlock{})
+	app.Commit()
+
+	return gInfo, res, err
+}
+
+// Move epochs to the future to avoid issues with minting
+func (chain *TestChain) MoveEpochsToTheFuture() {
+	epochsKeeper := chain.GetOsmosisApp().EpochsKeeper
+	ctx := chain.GetContext()
+	for _, epoch := range epochsKeeper.AllEpochInfos(ctx) {
+		epoch.StartTime = ctx.BlockTime().Add(time.Hour * 24 * 30)
+		epochsKeeper.DeleteEpochInfo(chain.GetContext(), epoch.Identifier)
+		_ = epochsKeeper.AddEpochInfo(ctx, epoch)
+	}
+}
+
+// GetOsmosisApp returns the current chain's app as an OsmosisApp
+func (chain *TestChain) GetOsmosisApp() *app.OsmosisApp {
+	v, _ := chain.App.(*app.OsmosisApp)
+	return v
+}

x/ibc-rate-limit/testutil/wasm.go

+package osmosisibctesting
+
+import (
+	"fmt"
+	"io/ioutil"
+
+	"github.com/stretchr/testify/require"
+
+	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/v3/modules/apps/transfer/types"
+	"github.com/osmosis-labs/osmosis/v12/x/ibc-rate-limit/types"
+	"github.com/stretchr/testify/suite"
+)
+
+func (chain *TestChain) StoreContractCode(suite *suite.Suite) {
+	osmosisApp := chain.GetOsmosisApp()
+
+	govKeeper := osmosisApp.GovKeeper
+	wasmCode, err := ioutil.ReadFile("./testdata/rate_limiter.wasm")
+	suite.Require().NoError(err)
+
+	addr := osmosisApp.AccountKeeper.GetModuleAddress(govtypes.ModuleName)
+	src := wasmtypes.StoreCodeProposalFixture(func(p *wasmtypes.StoreCodeProposal) {
+		p.RunAs = addr.String()
+		p.WASMByteCode = wasmCode
+	})
+
+	// when stored
+	storedProposal, err := govKeeper.SubmitProposal(chain.GetContext(), src, false)
+	suite.Require().NoError(err)
+
+	// and proposal execute
+	handler := govKeeper.Router().GetRoute(storedProposal.ProposalRoute())
+	err = handler(chain.GetContext(), storedProposal.GetContent())
+	suite.Require().NoError(err)
+}
+
+func (chain *TestChain) InstantiateContract(suite *suite.Suite, quotas string) sdk.AccAddress {
+	osmosisApp := chain.GetOsmosisApp()
+	transferModule := osmosisApp.AccountKeeper.GetModuleAddress(transfertypes.ModuleName)
+	govModule := osmosisApp.AccountKeeper.GetModuleAddress(govtypes.ModuleName)
+
+	initMsgBz := []byte(fmt.Sprintf(`{
+           "gov_module":  "%s",
+           "ibc_module":"%s",
+           "paths": [%s]
+        }`,
+		govModule, transferModule, quotas))
+
+	contractKeeper := wasmkeeper.NewDefaultPermissionKeeper(osmosisApp.WasmKeeper)
+	codeID := uint64(1)
+	creator := osmosisApp.AccountKeeper.GetModuleAddress(govtypes.ModuleName)
+	addr, _, err := contractKeeper.Instantiate(chain.GetContext(), codeID, creator, creator, initMsgBz, "rate limiting contract", nil)
+	suite.Require().NoError(err)
+	return addr
+}
+
+func (chain *TestChain) RegisterRateLimitingContract(addr []byte) {
+	addrStr, err := sdk.Bech32ifyAddressBytes("osmo", addr)
+	require.NoError(chain.T, err)
+	params, err := types.NewParams(addrStr)
+	require.NoError(chain.T, err)
+	osmosisApp := chain.GetOsmosisApp()
+	paramSpace, ok := osmosisApp.AppKeepers.ParamsKeeper.GetSubspace(types.ModuleName)
+	require.True(chain.T, ok)
+	paramSpace.SetParamSet(chain.GetContext(), &params)
+}

x/ibc-rate-limit/types/errors.go

@@ -5,8 +5,7 @@ import (
 )
 
 var (
-	RateLimitExceededMsg = "rate limit exceeded"
-	ErrRateLimitExceeded = sdkerrors.Register(ModuleName, 2, RateLimitExceededMsg)
+	ErrRateLimitExceeded = sdkerrors.Register(ModuleName, 2, "rate limit exceeded")
 	ErrBadMessage        = sdkerrors.Register(ModuleName, 3, "bad message")
 	ErrContractError     = sdkerrors.Register(ModuleName, 4, "contract error")
 )