diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml index f5f5301c31b5cb4ee0d9d09c198003116a04693a..50dc9addb06c842bce0250653d0b1071659e2d39 100644 --- a/.github/workflows/contracts.yml +++ b/.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 diff --git a/.gitignore b/.gitignore index 713ac5a10339e0b87df67f4da5fdfadcae495afb..ff99e2886b44efb0472db1c1eb1c8abec0e9e522 100644 --- a/.gitignore +++ b/.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 diff --git a/app/apptesting/events.go b/app/apptesting/events.go index 7d0a4d4dfdddd19663507de49912b4406931ecab..cdfae5028861208b0af3a85fca496187d92ce03e 100644 --- a/app/apptesting/events.go +++ b/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) } diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 7804dc34024b4eb7c4887ea1d7490437352cabba..83c53e3f2cd6d2cabde2b253f0b49ece129fb75c 100644 --- a/app/keepers/keepers.go +++ b/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 } diff --git a/tests/e2e/configurer/chain/commands.go b/tests/e2e/configurer/chain/commands.go index ee070b4b75452ff795b441510b829375ed328bd0..335d4924ecacaaf9d16718b116427d3829f59121 100644 --- a/tests/e2e/configurer/chain/commands.go +++ b/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)") diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 5c9b97e0ae8056b2556239dbb7775b06e2ac3c06..ec98a290877dcf388655a3df473a3713b5d005d6 100644 --- a/tests/e2e/e2e_test.go +++ b/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", ¶ms) + 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 { diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract_tests.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract_tests.rs index 16bc08802b072971eb1dbb887da57bb4eefbd1d4..fa5b99e49dacbea714a71d230b82136d047f9d20 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract_tests.rs +++ b/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 { diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs index dc40f708d1c0271ca0829bac7549b7c5ec1631e2..367180baf59e39d996ec6235b3e17d22c2503541 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs @@ -1,4 +1,4 @@ -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, }, diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs index 66a145b397dc04d66076f8df023fce805dc1a105..d5d76acb0e8a249eb5f8aaea69a7c39b9a7981af 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs +++ b/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); diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/packet.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/packet.rs new file mode 100644 index 0000000000000000000000000000000000000000..6bc5b8cfed1c2a4fb60ef80f0e1b6fc09df13ca0 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/packet.rs @@ -0,0 +1,64 @@ +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); + } +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs index 5237946487d0d84fd3676cf23b7f7e791f7a68c0..e28fc1004b7fb224e39490191911ee5e1d809163 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/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 { diff --git a/x/ibc-rate-limit/ibc_middleware_test.go b/x/ibc-rate-limit/ibc_middleware_test.go new file mode 100644 index 0000000000000000000000000000000000000000..497916d8b1addd61acb8a37b873731de1572de48 --- /dev/null +++ b/x/ibc-rate-limit/ibc_middleware_test.go @@ -0,0 +1,462 @@ +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))) +} diff --git a/x/ibc-rate-limit/ibc_module.go b/x/ibc-rate-limit/ibc_module.go index c1df7c9219f3bbb811751675476dea539c224790..433826dddacd260001e8236893d84107dad6a5eb 100644 --- a/x/ibc-rate-limit/ibc_module.go +++ b/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 diff --git a/x/ibc-rate-limit/ics4_wrapper.go b/x/ibc-rate-limit/ics4_wrapper.go index bdf7e935aaf8aae9d78d3dbb440e73ba7bb69b39..453de40a4fcbd1f5c677af7f7e9c9ac93fe37229 100644 --- a/x/ibc-rate-limit/ics4_wrapper.go +++ b/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) } diff --git a/x/ibc-rate-limit/rate_limit.go b/x/ibc-rate-limit/rate_limit.go index 665f04b299028c939c3e7022462a39f726ae7d4a..5c91e5ffeedeea7d26aaf99c7e45c9bf19071a35 100644 --- a/x/ibc-rate-limit/rate_limit.go +++ b/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 + } } diff --git a/x/ibc-rate-limit/testdata/rate_limiter.wasm b/x/ibc-rate-limit/testdata/rate_limiter.wasm index caf63c41459ca8361691cf27ae6c738682946e40..e19651209c4009fd4bd73bae23f49c69424e91d3 100644 Binary files a/x/ibc-rate-limit/testdata/rate_limiter.wasm and b/x/ibc-rate-limit/testdata/rate_limiter.wasm differ diff --git a/x/ibc-rate-limit/testutil/chain.go b/x/ibc-rate-limit/testutil/chain.go new file mode 100644 index 0000000000000000000000000000000000000000..3ab9c26f0e24fe6b3841db770705f1515c9885e4 --- /dev/null +++ b/x/ibc-rate-limit/testutil/chain.go @@ -0,0 +1,96 @@ +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 +} diff --git a/x/ibc-rate-limit/testutil/wasm.go b/x/ibc-rate-limit/testutil/wasm.go new file mode 100644 index 0000000000000000000000000000000000000000..2beabb9c02ae12fe732b7cd1f497d4a5166fd278 --- /dev/null +++ b/x/ibc-rate-limit/testutil/wasm.go @@ -0,0 +1,70 @@ +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(), ¶ms) +} diff --git a/x/ibc-rate-limit/types/errors.go b/x/ibc-rate-limit/types/errors.go index 67d81abeb79f4b94762e9c2ec72cb74b49a4c96c..5394ce11e3d97a6907f1885a29fc119ce38f0c8f 100644 --- a/x/ibc-rate-limit/types/errors.go +++ b/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") )