From 0a2656cf9efe6297843b4f9d5ff1634497585235 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Mon, 15 Aug 2022 08:58:11 +0200 Subject: [PATCH 01/29] added ibc-rate-limiting contract --- x/ibc-rate-limit/.beaker/state.json | 1 + x/ibc-rate-limit/.gitignore | 21 + x/ibc-rate-limit/Cargo.toml | 16 + x/ibc-rate-limit/contracts/.gitkeep | 0 .../contracts/rate-limiter/.cargo/config | 3 + .../contracts/rate-limiter/.gitignore | 15 + .../contracts/rate-limiter/Cargo.toml | 53 ++ .../contracts/rate-limiter/src/contract.rs | 501 ++++++++++++++++++ .../contracts/rate-limiter/src/error.rs | 20 + .../contracts/rate-limiter/src/helpers.rs | 52 ++ .../rate-limiter/src/integration_tests.rs | 273 ++++++++++ .../contracts/rate-limiter/src/lib.rs | 9 + .../contracts/rate-limiter/src/management.rs | 238 +++++++++ .../contracts/rate-limiter/src/msg.rs | 69 +++ .../contracts/rate-limiter/src/state.rs | 149 ++++++ 15 files changed, 1420 insertions(+) create mode 100644 x/ibc-rate-limit/.beaker/state.json create mode 100644 x/ibc-rate-limit/.gitignore create mode 100644 x/ibc-rate-limit/Cargo.toml create mode 100644 x/ibc-rate-limit/contracts/.gitkeep create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/.cargo/config create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/.gitignore create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/error.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/management.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/state.rs diff --git a/x/ibc-rate-limit/.beaker/state.json b/x/ibc-rate-limit/.beaker/state.json new file mode 100644 index 00000000000..9e26dfeeb6e --- /dev/null +++ b/x/ibc-rate-limit/.beaker/state.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/x/ibc-rate-limit/.gitignore b/x/ibc-rate-limit/.gitignore new file mode 100644 index 00000000000..0814c1f8964 --- /dev/null +++ b/x/ibc-rate-limit/.gitignore @@ -0,0 +1,21 @@ +# Generated by Cargo +# will have compiled files and executables +debug/ +target/ + +# Generated by rust-optimizer +artifacts/ + +# Remove Cargo.lock from gitignore if creating an executable, leave it for libraries +# More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html +Cargo.lock + +# These are backup files generated by rustfmt +**/*.rs.bk + +# MSVC Windows builds of rustc generate these, which store debugging information +*.pdb + + +# Ignores local beaker state +**/state.local.json diff --git a/x/ibc-rate-limit/Cargo.toml b/x/ibc-rate-limit/Cargo.toml new file mode 100644 index 00000000000..9e4bf04d415 --- /dev/null +++ b/x/ibc-rate-limit/Cargo.toml @@ -0,0 +1,16 @@ +[workspace] + +members = [ + 'contracts/*', +] + +[profile.release] +codegen-units = 1 +debug = false +debug-assertions = false +incremental = false +lto = true +opt-level = 3 +overflow-checks = true +panic = 'abort' +rpath = false diff --git a/x/ibc-rate-limit/contracts/.gitkeep b/x/ibc-rate-limit/contracts/.gitkeep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/x/ibc-rate-limit/contracts/rate-limiter/.cargo/config b/x/ibc-rate-limit/contracts/rate-limiter/.cargo/config new file mode 100644 index 00000000000..f31de6c2a75 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/.cargo/config @@ -0,0 +1,3 @@ +[alias] +wasm = "build --release --target wasm32-unknown-unknown" +unit-test = "test --lib" diff --git a/x/ibc-rate-limit/contracts/rate-limiter/.gitignore b/x/ibc-rate-limit/contracts/rate-limiter/.gitignore new file mode 100644 index 00000000000..dfdaaa6bcda --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/.gitignore @@ -0,0 +1,15 @@ +# Build results +/target + +# Cargo+Git helper file (https://github.com/rust-lang/cargo/blob/0.44.1/src/cargo/sources/git/utils.rs#L320-L327) +.cargo-ok + +# Text file backups +**/*.rs.bk + +# macOS +.DS_Store + +# IDEs +*.iml +.idea diff --git a/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml b/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml new file mode 100644 index 00000000000..a94d596a72c --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml @@ -0,0 +1,53 @@ +[package] +name = "rate-limiter" +version = "0.1.0" +authors = ["Nicolas Lara "] +edition = "2018" + +exclude = [ + # Those files are rust-optimizer artifacts. You might want to commit them for convenience but they should not be part of the source code publication. + "contract.wasm", + "hash.txt", +] + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[lib] +crate-type = ["cdylib", "rlib"] + +[profile.release] +opt-level = 3 +debug = false +rpath = false +lto = true +debug-assertions = false +codegen-units = 1 +panic = 'abort' +incremental = false +overflow-checks = true + +[features] +# for more explicit tests, cargo test --features=backtraces +backtraces = ["cosmwasm-std/backtraces"] +# use library feature to disable all instantiate/execute/query exports +library = [] + +[package.metadata.scripts] +optimize = """docker run --rm -v "$(pwd)":/code \ + --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target \ + --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \ + cosmwasm/rust-optimizer:0.12.6 +""" + +[dependencies] +cosmwasm-std = "1.0.0" +cosmwasm-storage = "1.0.0" +cw-storage-plus = "0.13.2" +cw2 = "0.13.2" +schemars = "0.8.8" +serde = { version = "1.0.137", default-features = false, features = ["derive"] } +thiserror = { version = "1.0.31" } + +[dev-dependencies] +cosmwasm-schema = "1.0.0" +cw-multi-test = "0.13.2" diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs new file mode 100644 index 00000000000..821fc2ad514 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -0,0 +1,501 @@ +#[cfg(not(feature = "library"))] +use cosmwasm_std::entry_point; +use cosmwasm_std::{ + to_binary, Addr, Binary, Deps, DepsMut, Env, MessageInfo, Response, StdResult, Timestamp, +}; +use cw2::set_contract_version; + +use crate::error::ContractError; +use crate::management::{ + add_new_channels, try_add_channel, try_remove_channel, try_reset_channel_quota, +}; +use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg}; +use crate::state::{ChannelFlow, FlowType, CHANNEL_FLOWS, GOVMODULE, IBCMODULE}; + +// version info for migration info +const CONTRACT_NAME: &str = "crates.io:rate-limiter"; +const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn instantiate( + deps: DepsMut, + env: Env, + _info: MessageInfo, + msg: InstantiateMsg, +) -> Result { + set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; + IBCMODULE.save(deps.storage, &msg.ibc_module)?; + GOVMODULE.save(deps.storage, &msg.gov_module)?; + + add_new_channels(deps, msg.channels, env.block.time)?; + + Ok(Response::new() + .add_attribute("method", "instantiate") + .add_attribute("ibc_module", msg.ibc_module.to_string()) + .add_attribute("gov_module", msg.gov_module.to_string())) +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn execute( + deps: DepsMut, + env: Env, + info: MessageInfo, + msg: ExecuteMsg, +) -> Result { + match msg { + ExecuteMsg::SendPacket { + channel_id, + channel_value, + funds, + } => try_transfer( + deps, + info.sender, + channel_id, + channel_value, + funds, + FlowType::Out, + env.block.time, + ), + ExecuteMsg::RecvPacket { + channel_id, + channel_value, + funds, + } => try_transfer( + deps, + info.sender, + channel_id, + channel_value, + funds, + FlowType::In, + env.block.time, + ), + ExecuteMsg::AddChannel { channel_id, quotas } => { + try_add_channel(deps, info.sender, channel_id, quotas, env.block.time) + } + ExecuteMsg::RemoveChannel { channel_id } => { + try_remove_channel(deps, info.sender, channel_id) + } + ExecuteMsg::ResetChannelQuota { + channel_id, + quota_id, + } => try_reset_channel_quota(deps, info.sender, channel_id, quota_id, env.block.time), + } +} + +pub struct ChannelFlowResponse { + pub channel_flow: ChannelFlow, + pub used: u128, + pub max: u128, +} + +pub fn try_transfer( + deps: DepsMut, + sender: Addr, + channel_id: String, + channel_value: u128, + funds: u128, + direction: FlowType, + now: Timestamp, +) -> Result { + // Only the IBCMODULE can execute transfers + let ibc_module = IBCMODULE.load(deps.storage)?; + if sender != ibc_module { + return Err(ContractError::Unauthorized {}); + } + + let channels = CHANNEL_FLOWS.may_load(deps.storage, &channel_id)?; + + let configured = match channels { + None => false, + Some(ref x) if x.len() == 0 => false, + _ => true, + }; + + if !configured { + // No Quota configured for the current channel. Allowing all messages. + return Ok(Response::new() + .add_attribute("method", "try_transfer") + .add_attribute("channel_id", channel_id) + .add_attribute("quota", "none")); + } + + let mut channels = channels.unwrap(); + + let results: Result, _> = channels + .iter_mut() + .map(|channel| { + let max = channel.quota.capacity_at(&channel_value, &direction); + if channel.flow.is_expired(now) { + channel.flow.expire(now, channel.quota.duration) + } + channel.flow.add_flow(direction.clone(), funds); + + let balance = channel.flow.balance(); + if balance > max { + return Err(ContractError::RateLimitExceded { + channel: channel_id.to_string(), + reset: channel.flow.period_end, + }); + }; + Ok(ChannelFlowResponse { + channel_flow: ChannelFlow { + quota: channel.quota.clone(), + flow: channel.flow.clone(), + }, + used: balance, + max, + }) + }) + .collect(); + let results = results?; + + CHANNEL_FLOWS.save( + deps.storage, + &channel_id, + &results.iter().map(|r| r.channel_flow.clone()).collect(), + )?; + + let response = Response::new() + .add_attribute("method", "try_transfer") + .add_attribute("channel_id", channel_id); + + // Adding the attributes from each quota to the response + results.iter().fold(Ok(response), |acc, result| { + Ok(acc? + .add_attribute( + format!("{}_used", result.channel_flow.quota.name), + result.used.to_string(), + ) + .add_attribute( + format!("{}_max", result.channel_flow.quota.name), + result.max.to_string(), + ) + .add_attribute( + format!("{}_period_end", result.channel_flow.quota.name), + result.channel_flow.flow.period_end.to_string(), + )) + }) +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { + match msg { + QueryMsg::GetQuotas { channel_id } => get_quotas(deps, channel_id), + } +} + +fn get_quotas(deps: Deps, channel_id: impl Into) -> StdResult { + to_binary(&CHANNEL_FLOWS.load(deps.storage, &channel_id.into())?) +} + +#[cfg(test)] +mod tests { + use super::*; + use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info}; + use cosmwasm_std::{from_binary, Addr, Attribute}; + + use crate::helpers::tests::verify_query_response; + use crate::msg::{Channel, QuotaMsg}; + use crate::state::RESET_TIME_WEEKLY; + + const IBC_ADDR: &str = "IBC_MODULE"; + const GOV_ADDR: &str = "GOV_MODULE"; + + #[test] + fn proper_instantiation() { + let mut deps = mock_dependencies(); + + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels: vec![], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + // we can just call .unwrap() to assert this was a success + let res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap(); + assert_eq!(0, res.messages.len()); + + // The ibc and gov modules are properly stored + assert_eq!(IBCMODULE.load(deps.as_ref().storage).unwrap(), IBC_ADDR); + assert_eq!(GOVMODULE.load(deps.as_ref().storage).unwrap(), GOV_ADDR); + } + + #[test] + fn permissions() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("Weekly", RESET_TIME_WEEKLY, 10, 10); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels: vec![Channel { + name: "channel".to_string(), + quotas: vec![quota], + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + + // This succeeds + execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + + let info = mock_info("SomeoneElse", &vec![]); + + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + let err = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap_err(); + assert!(matches!(err, ContractError::Unauthorized { .. })); + } + + #[test] + fn consume_allowance() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels: vec![Channel { + name: "channel".to_string(), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + let info = mock_info(IBC_ADDR, &vec![]); + let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + let Attribute { key, value } = &res.attributes[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "300"); + + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + let err = execute(deps.as_mut(), mock_env(), info, msg).unwrap_err(); + assert!(matches!(err, ContractError::RateLimitExceded { .. })); + } + + #[test] + fn symetric_flows_dont_consume_allowance() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels: vec![Channel { + name: "channel".to_string(), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + + let info = mock_info(IBC_ADDR, &vec![]); + let send_msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + let recv_msg = ExecuteMsg::RecvPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + + let res = execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "300"); + + let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "0"); + + // We can still use the channel. Even if we have sent more than the + // allowance through the channel (900 > 3000*.1), the current "balance" + // of inflow vs outflow is still lower than the channel's capacity/quota + let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "300"); + + let err = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap_err(); + + assert!(matches!(err, ContractError::RateLimitExceded { .. })); + //assert_eq!(18, value.count); + } + + #[test] + fn asymetric_quotas() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 1); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels: vec![Channel { + name: "channel".to_string(), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + + // Sending 2% + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 60, + }; + let info = mock_info(IBC_ADDR, &vec![]); + let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + let Attribute { key, value } = &res.attributes[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "60"); + + // Sending 1% more. Allowed, as sending has a 10% allowance + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 30, + }; + + let info = mock_info(IBC_ADDR, &vec![]); + let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + let Attribute { key, value } = &res.attributes[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "90"); + + // Receiving 1% should fail. 3% already executed through the channel + let recv_msg = ExecuteMsg::RecvPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 30, + }; + + let err = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap_err(); + assert!(matches!(err, ContractError::RateLimitExceded { .. })); + } + + #[test] + fn query_state() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels: vec![Channel { + name: "channel".to_string(), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let env = mock_env(); + let _res = instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); + + let query_msg = QueryMsg::GetQuotas { + channel_id: "channel".to_string(), + }; + + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + assert_eq!(value[0].quota.name, "weekly"); + assert_eq!(value[0].quota.max_percentage_send, 10); + assert_eq!(value[0].quota.max_percentage_recv, 10); + assert_eq!(value[0].quota.duration, RESET_TIME_WEEKLY); + assert_eq!(value[0].flow.inflow, 0); + assert_eq!(value[0].flow.outflow, 0); + assert_eq!( + value[0].flow.period_end, + env.block.time.plus_seconds(RESET_TIME_WEEKLY) + ); + + let info = mock_info(IBC_ADDR, &vec![]); + let send_msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); + + let recv_msg = ExecuteMsg::RecvPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 30, + }; + execute(deps.as_mut(), mock_env(), info, recv_msg.clone()).unwrap(); + + // Query + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + verify_query_response( + &value[0], + "weekly", + (10, 10), + RESET_TIME_WEEKLY, + 30, + 300, + env.block.time.plus_seconds(RESET_TIME_WEEKLY), + ); + } + + #[test] + fn bad_quotas() { + let mut deps = mock_dependencies(); + + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels: vec![Channel { + name: "channel".to_string(), + quotas: vec![QuotaMsg { + name: "bad_quota".to_string(), + duration: 200, + send_recv: (5000, 101), + }], + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + // we can just call .unwrap() to assert this was a success + let env = mock_env(); + instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); + + // If a quota is higher than 100%, we set it to 100% + let query_msg = QueryMsg::GetQuotas { + channel_id: "channel".to_string(), + }; + let res = query(deps.as_ref(), env.clone(), query_msg).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + verify_query_response( + &value[0], + "bad_quota", + (100, 100), + 200, + 0, + 0, + env.block.time.plus_seconds(200), + ); + } +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs new file mode 100644 index 00000000000..3c58ce21bc9 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs @@ -0,0 +1,20 @@ +use cosmwasm_std::{StdError, Timestamp}; +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum ContractError { + #[error("{0}")] + Std(#[from] StdError), + + #[error("Unauthorized")] + Unauthorized {}, + + #[error("IBC Rate Limit exceded for channel {channel:?}. Try again after {reset:?}")] + RateLimitExceded { channel: String, reset: Timestamp }, + + #[error("Quota {quota_id} not found for channel {channel_id}")] + QuotaNotFound { + quota_id: String, + channel_id: String, + }, +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs new file mode 100644 index 00000000000..82f4f1168ba --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs @@ -0,0 +1,52 @@ +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use cosmwasm_std::{to_binary, Addr, CosmosMsg, StdResult, WasmMsg}; + +use crate::msg::ExecuteMsg; + +/// CwTemplateContract is a wrapper around Addr that provides a lot of helpers +/// for working with this. +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +pub struct RateLimitingContract(pub Addr); + +impl RateLimitingContract { + pub fn addr(&self) -> Addr { + self.0.clone() + } + + pub fn call>(&self, msg: T) -> StdResult { + let msg = to_binary(&msg.into())?; + Ok(WasmMsg::Execute { + contract_addr: self.addr().into(), + msg, + funds: vec![], + } + .into()) + } +} + +#[cfg(test)] +pub mod tests { + use cosmwasm_std::Timestamp; + + use crate::state::ChannelFlow; + + pub fn verify_query_response( + value: &ChannelFlow, + quota_name: &str, + send_recv: (u32, u32), + duration: u64, + inflow: u128, + outflow: u128, + period_end: Timestamp, + ) { + assert_eq!(value.quota.name, quota_name); + assert_eq!(value.quota.max_percentage_send, send_recv.0); + assert_eq!(value.quota.max_percentage_recv, send_recv.1); + assert_eq!(value.quota.duration, duration); + assert_eq!(value.flow.inflow, inflow); + assert_eq!(value.flow.outflow, outflow); + assert_eq!(value.flow.period_end, period_end); + } +} 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 new file mode 100644 index 00000000000..26f48cea159 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs @@ -0,0 +1,273 @@ +#[cfg(test)] +mod tests { + use crate::helpers::RateLimitingContract; + use crate::msg::{Channel, InstantiateMsg}; + use cosmwasm_std::{Addr, Coin, Empty, Uint128}; + use cw_multi_test::{App, AppBuilder, Contract, ContractWrapper, Executor}; + + pub fn contract_template() -> Box> { + let contract = ContractWrapper::new( + crate::contract::execute, + crate::contract::instantiate, + crate::contract::query, + ); + Box::new(contract) + } + + const USER: &str = "USER"; + const IBC_ADDR: &str = "IBC_MODULE"; + const GOV_ADDR: &str = "GOV_MODULE"; + const NATIVE_DENOM: &str = "nosmo"; + + fn mock_app() -> App { + AppBuilder::new().build(|router, _, storage| { + router + .bank + .init_balance( + storage, + &Addr::unchecked(USER), + vec![Coin { + denom: NATIVE_DENOM.to_string(), + amount: Uint128::new(1_000), + }], + ) + .unwrap(); + }) + } + + fn proper_instantiate(channels: Vec) -> (App, RateLimitingContract) { + let mut app = mock_app(); + let cw_template_id = app.store_code(contract_template()); + + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + channels, + }; + + let cw_template_contract_addr = app + .instantiate_contract( + cw_template_id, + Addr::unchecked(GOV_ADDR), + &msg, + &[], + "test", + None, + ) + .unwrap(); + + let cw_template_contract = RateLimitingContract(cw_template_contract_addr); + + (app, cw_template_contract) + } + + mod expiration { + use cosmwasm_std::Attribute; + + use super::*; + use crate::{ + msg::{Channel, ExecuteMsg, QuotaMsg}, + state::{RESET_TIME_DAILY, RESET_TIME_MONTHLY, RESET_TIME_WEEKLY}, + }; + + #[test] + fn expiration() { + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + + let (mut app, cw_template_contract) = proper_instantiate(vec![Channel { + name: "channel".to_string(), + quotas: vec![quota], + }]); + + // Using all the allowance + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + let Attribute { key, value } = &res.custom_attrs(1)[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[3]; + assert_eq!(key, "weekly_max"); + assert_eq!(value, "300"); + + // Another packet is rate limited + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // TODO: how do we check the error type here? + println!("{err:?}"); + + // ... Time passes + app.update_block(|b| { + b.height += 1000; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // Sending the packet should work now + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 3_000, + funds: 300, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + let Attribute { key, value } = &res.custom_attrs(1)[2]; + assert_eq!(key, "weekly_used"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[3]; + assert_eq!(key, "weekly_max"); + assert_eq!(value, "300"); + } + + #[test] + fn multiple_quotas() { + let quotas = vec![ + QuotaMsg::new("daily", RESET_TIME_DAILY, 1, 1), + QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), + QuotaMsg::new("monthly", RESET_TIME_MONTHLY, 5, 5), + ]; + + let (mut app, cw_template_contract) = proper_instantiate(vec![Channel { + name: "channel".to_string(), + quotas, + }]); + + // Sending 1% to use the daily allowance + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + println!("{res:?}"); + + // Another packet is rate limited + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // Sending the packet should work now + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + // Do that for 4 more days + for _ in 1..4 { + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // Sending the packet should work now + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + } + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // We now have exceeded the weekly limit! Even if the daily limit allows us, the weekly doesn't + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // We can still can't send because the weekly and monthly limits are the same + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // Waiting a week again, doesn't help!! + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // We can still can't send because the monthly limit hasn't passed + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // Only after two more weeks we can send again + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds((RESET_TIME_WEEKLY * 2) + 1) // Two weeks + }); + + println!("{:?}", app.block_info()); + let msg = ExecuteMsg::SendPacket { + channel_id: "channel".to_string(), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + } + } +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs new file mode 100644 index 00000000000..573d76512d7 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs @@ -0,0 +1,9 @@ +pub mod contract; +mod error; +pub mod helpers; +pub mod integration_tests; +pub mod management; +pub mod msg; +pub mod state; + +pub use crate::error::ContractError; diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs new file mode 100644 index 00000000000..b1ba33ce35b --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs @@ -0,0 +1,238 @@ +use crate::msg::{Channel, QuotaMsg}; +use crate::state::{ChannelFlow, Flow, CHANNEL_FLOWS, GOVMODULE, IBCMODULE}; +use crate::ContractError; +use cosmwasm_std::{Addr, DepsMut, Response, Timestamp}; + +pub fn add_new_channels( + deps: DepsMut, + channels: Vec, + now: Timestamp, +) -> Result<(), ContractError> { + for channel in channels { + CHANNEL_FLOWS.save( + deps.storage, + &channel.name, + &channel + .quotas + .iter() + .map(|q| ChannelFlow { + quota: q.into(), + flow: Flow::new(0_u128, 0_u128, now, q.duration), + }) + .collect(), + )? + } + Ok(()) +} + +pub fn try_add_channel( + deps: DepsMut, + sender: Addr, + channel_id: String, + quotas: Vec, + now: Timestamp, +) -> Result { + let ibc_module = IBCMODULE.load(deps.storage)?; + let gov_module = GOVMODULE.load(deps.storage)?; + if sender != ibc_module && sender != gov_module { + return Err(ContractError::Unauthorized {}); + } + add_new_channels( + deps, + vec![Channel { + name: channel_id.to_string(), + quotas, + }], + now, + )?; + + Ok(Response::new() + .add_attribute("method", "try_add_channel") + .add_attribute("channel_id", channel_id)) +} + +pub fn try_remove_channel( + deps: DepsMut, + sender: Addr, + channel_id: String, +) -> Result { + let ibc_module = IBCMODULE.load(deps.storage)?; + let gov_module = GOVMODULE.load(deps.storage)?; + if sender != ibc_module && sender != gov_module { + return Err(ContractError::Unauthorized {}); + } + CHANNEL_FLOWS.remove(deps.storage, &channel_id); + Ok(Response::new() + .add_attribute("method", "try_remove_channel") + .add_attribute("channel_id", channel_id)) +} + +pub fn try_reset_channel_quota( + deps: DepsMut, + sender: Addr, + channel_id: String, + quota_id: String, + now: Timestamp, +) -> Result { + let gov_module = GOVMODULE.load(deps.storage)?; + if sender != gov_module { + return Err(ContractError::Unauthorized {}); + } + + CHANNEL_FLOWS.update( + deps.storage, + &channel_id.clone(), + |maybe_flows| match maybe_flows { + None => Err(ContractError::QuotaNotFound { + quota_id, + channel_id: channel_id.clone(), + }), + Some(mut flows) => { + flows.iter_mut().for_each(|channel| { + if channel.quota.name == channel_id.as_ref() { + channel.flow.expire(now, channel.quota.duration) + } + }); + Ok(flows) + } + }, + )?; + + Ok(Response::new() + .add_attribute("method", "try_reset_channel") + .add_attribute("channel_id", channel_id)) +} + +#[cfg(test)] +mod tests { + + use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info}; + use cosmwasm_std::{from_binary, Addr, StdError}; + + use crate::contract::{execute, query}; + use crate::helpers::tests::verify_query_response; + use crate::msg::{ExecuteMsg, QueryMsg, QuotaMsg}; + use crate::state::{ChannelFlow, GOVMODULE, IBCMODULE}; + + const IBC_ADDR: &str = "IBC_MODULE"; + const GOV_ADDR: &str = "GOV_MODULE"; + + #[test] + fn management_add_and_remove_channel() { + let mut deps = mock_dependencies(); + IBCMODULE + .save(deps.as_mut().storage, &Addr::unchecked(IBC_ADDR)) + .unwrap(); + GOVMODULE + .save(deps.as_mut().storage, &Addr::unchecked(GOV_ADDR)) + .unwrap(); + + let msg = ExecuteMsg::AddChannel { + channel_id: "channel".to_string(), + quotas: vec![QuotaMsg { + name: "daily".to_string(), + duration: 1600, + send_recv: (3, 5), + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + let env = mock_env(); + let res = execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + assert_eq!(0, res.messages.len()); + + let query_msg = QueryMsg::GetQuotas { + channel_id: "channel".to_string(), + }; + + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + + let value: Vec = from_binary(&res).unwrap(); + verify_query_response( + &value[0], + "daily", + (3, 5), + 1600, + 0, + 0, + env.block.time.plus_seconds(1600), + ); + + assert_eq!(value.len(), 1); + + // Add another channel + let msg = ExecuteMsg::AddChannel { + channel_id: "channel2".to_string(), + quotas: vec![QuotaMsg { + name: "daily".to_string(), + duration: 1600, + send_recv: (3, 5), + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + let env = mock_env(); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + // remove the first one + let msg = ExecuteMsg::RemoveChannel { + channel_id: "channel".to_string(), + }; + + let info = mock_info(IBC_ADDR, &vec![]); + let env = mock_env(); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + // The channel is not there anymore + let err = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap_err(); + assert!(matches!(err, StdError::NotFound { .. })); + + // The second channel is still there + let query_msg = QueryMsg::GetQuotas { + channel_id: "channel2".to_string(), + }; + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + assert_eq!(value.len(), 1); + verify_query_response( + &value[0], + "daily", + (3, 5), + 1600, + 0, + 0, + env.block.time.plus_seconds(1600), + ); + + // Channels are overriden if they share a name + let msg = ExecuteMsg::AddChannel { + channel_id: "channel2".to_string(), + quotas: vec![QuotaMsg { + name: "different".to_string(), + duration: 5000, + send_recv: (50, 30), + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + let env = mock_env(); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + let query_msg = QueryMsg::GetQuotas { + channel_id: "channel2".to_string(), + }; + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + assert_eq!(value.len(), 1); + println!("{value:?}"); + verify_query_response( + &value[0], + "different", + (50, 30), + 5000, + 0, + 0, + env.block.time.plus_seconds(5000), + ); + } +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs new file mode 100644 index 00000000000..35f2812db57 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -0,0 +1,69 @@ +use cosmwasm_std::Addr; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +pub struct Channel { + pub name: String, + pub quotas: Vec, +} + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +pub struct QuotaMsg { + pub name: String, + pub duration: u64, + pub send_recv: (u32, u32), +} + +impl QuotaMsg { + pub fn new(name: &str, seconds: u64, send_percentage: u32, recv_percentage: u32) -> Self { + QuotaMsg { + name: name.to_string(), + duration: seconds, + send_recv: (send_percentage, recv_percentage), + } + } +} + +/// Initialize the contract with the address of the IBC module and any existing channels. +/// Only the ibc module is allowed to execute actions on this contract +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +pub struct InstantiateMsg { + pub gov_module: Addr, + pub ibc_module: Addr, + pub channels: Vec, +} + +/// The caller (IBC module) is responsibble for correctly calculating the funds +/// being sent through the channel +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum ExecuteMsg { + SendPacket { + channel_id: String, + channel_value: u128, + funds: u128, + }, + RecvPacket { + channel_id: String, + channel_value: u128, + funds: u128, + }, + AddChannel { + channel_id: String, + quotas: Vec, + }, + RemoveChannel { + channel_id: String, + }, + ResetChannelQuota { + channel_id: String, + quota_id: String, + }, +} + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum QueryMsg { + GetQuotas { channel_id: String }, +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs new file mode 100644 index 00000000000..adaa323aad6 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -0,0 +1,149 @@ +use cosmwasm_std::{Addr, Timestamp}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use std::cmp; + +use cw_storage_plus::{Item, Map}; + +use crate::msg::QuotaMsg; + +pub const RESET_TIME_DAILY: u64 = 60 * 60 * 24; +pub const RESET_TIME_WEEKLY: u64 = 60 * 60 * 24 * 7; +pub const RESET_TIME_MONTHLY: u64 = 60 * 60 * 24 * 30; + +#[derive(Debug, Clone)] +pub enum FlowType { + In, + Out, +} + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema, Copy)] +pub struct Flow { + pub inflow: u128, + pub outflow: u128, + pub period_end: Timestamp, +} + +impl Flow { + pub fn new( + inflow: impl Into, + outflow: impl Into, + now: Timestamp, + duration: u64, + ) -> Self { + Self { + inflow: inflow.into(), + outflow: outflow.into(), + period_end: now.plus_seconds(duration), + } + } + + pub fn balance(&self) -> u128 { + self.inflow.abs_diff(self.outflow) + } + + pub fn is_expired(&self, now: Timestamp) -> bool { + self.period_end < now + } + + // Mutating methods + pub fn expire(&mut self, now: Timestamp, duration: u64) { + self.inflow = 0; + self.outflow = 0; + self.period_end = now.plus_seconds(duration); + } + + pub fn add_flow(&mut self, direction: FlowType, value: u128) { + match direction { + FlowType::In => self.inflow = self.inflow.saturating_add(value), + FlowType::Out => self.outflow = self.outflow.saturating_add(value), + } + } +} + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +pub struct Quota { + pub name: String, + pub max_percentage_send: u32, + pub max_percentage_recv: u32, + pub duration: u64, +} + +impl Quota { + /// Calculates the max capacity based on the total value of the channel + pub fn capacity_at(&self, total_value: &u128, direction: &FlowType) -> u128 { + let max_percentage = match direction { + FlowType::In => self.max_percentage_recv, + FlowType::Out => self.max_percentage_send, + }; + total_value * (max_percentage as u128) / 100_u128 + } +} + +impl From<&QuotaMsg> for Quota { + fn from(msg: &QuotaMsg) -> Self { + let send_recv = ( + cmp::min(msg.send_recv.0, 100), + cmp::min(msg.send_recv.1, 100), + ); + Quota { + name: msg.name.clone(), + max_percentage_send: send_recv.0, + max_percentage_recv: send_recv.1, + duration: msg.duration, + } + } +} + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +pub struct ChannelFlow { + pub quota: Quota, + pub flow: Flow, +} + +/// Only this module can manage the contract +pub const GOVMODULE: Item = Item::new("gov_module"); +/// Only this module can execute transfers +pub const IBCMODULE: Item = Item::new("ibc_module"); +// For simplicity, the map keys (ibc channel) refers to the "host" channel on the +// osmosis side. This means that on PacketSend it will refer to the source +// channel while on PacketRecv it refers to the destination channel. +// +// It is the responsibility of the go module to pass the appropriate channel +// when sending the messages +pub const CHANNEL_FLOWS: Map<&str, Vec> = Map::new("flow"); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn flow() { + let epoch = Timestamp::from_seconds(0); + let mut flow = Flow::new(0_u32, 0_u32, epoch, RESET_TIME_WEEKLY); + + assert!(!flow.is_expired(epoch)); + assert!(!flow.is_expired(epoch.plus_seconds(RESET_TIME_DAILY))); + assert!(!flow.is_expired(epoch.plus_seconds(RESET_TIME_WEEKLY))); + assert!(flow.is_expired(epoch.plus_seconds(RESET_TIME_WEEKLY).plus_nanos(1))); + + assert_eq!(flow.balance(), 0_u128); + flow.add_flow(FlowType::In, 5); + assert_eq!(flow.balance(), 5_u128); + flow.add_flow(FlowType::Out, 2); + assert_eq!(flow.balance(), 3_u128); + // Adding flow doesn't affect expiration + assert!(!flow.is_expired(epoch.plus_seconds(RESET_TIME_DAILY))); + + flow.expire(epoch.plus_seconds(RESET_TIME_WEEKLY), RESET_TIME_WEEKLY); + assert_eq!(flow.balance(), 0_u128); + assert_eq!(flow.inflow, 0_u128); + assert_eq!(flow.outflow, 0_u128); + assert_eq!(flow.period_end, epoch.plus_seconds(RESET_TIME_WEEKLY * 2)); + + // Expiration has moved + assert!(!flow.is_expired(epoch.plus_seconds(RESET_TIME_WEEKLY).plus_nanos(1))); + assert!(!flow.is_expired(epoch.plus_seconds(RESET_TIME_WEEKLY * 2))); + assert!(flow.is_expired(epoch.plus_seconds(RESET_TIME_WEEKLY * 2).plus_nanos(1))); + } +} From a6cf294173fd49ec6ed7b879b941c0694475b932 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Mon, 15 Aug 2022 12:40:19 +0200 Subject: [PATCH 02/29] added cosmwasm workflow --- .github/workflows/contracts.yml | 67 +++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 .github/workflows/contracts.yml diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml new file mode 100644 index 00000000000..29786ea7317 --- /dev/null +++ b/.github/workflows/contracts.yml @@ -0,0 +1,67 @@ +name: Cosmwasm Contracts +on: + pull_request: + push: + + +jobs: + test: + name: Test Suite + runs-on: ubuntu-latest + strategy: + matrix: + workdir: [./x/ibc-rate-limit] +# output: [./x/ibc-rate-limit/testdata/rate_limit.wasm] + + steps: + - name: Checkout sources + uses: actions/checkout@v2 + + - name: Install toolchain + uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + target: wasm32-unknown-unknown + + - name: Build + working-directory: ${{ matrix.workdir }} + run: > + rustup target add wasm32-unknown-unknown; + cargo build --release --target wasm32-unknown-unknown + + - name: Test + working-directory: ${{ matrix.workdir }} + run: > + cargo test + +# - name: Check Test Data +# working-directory: ${{ matrix.workdir }} +# if: ${{ matrix.output != null }} +# run: > +# ls ${{ matrix.output }}; +# ls ${{ matrix.output }}/artifacts; +# diff ${{ matrix.output }} ./artifacts/*.wasm + + + lints: + name: Cosmwasm Lints + runs-on: ubuntu-latest + strategy: + matrix: + workdir: [./x/ibc-rate-limit] + + steps: + - name: Checkout sources + uses: actions/checkout@v2 + + - name: Install toolchain + uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + components: rustfmt + target: wasm32-unknown-unknown + + - name: Format + working-directory: ${{ matrix.workdir }} + run: > + cargo fmt --all -- --check From 15cbcec3001ce973910dd9b768dfcf1258eb4b8c Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Tue, 16 Aug 2022 11:13:48 +0200 Subject: [PATCH 03/29] added doc comments --- .../contracts/rate-limiter/src/contract.rs | 7 ++- .../rate-limiter/src/integration_tests.rs | 8 +-- .../contracts/rate-limiter/src/state.rs | 51 +++++++++++++++---- 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 821fc2ad514..1da17deb175 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -9,7 +9,7 @@ use crate::error::ContractError; use crate::management::{ add_new_channels, try_add_channel, try_remove_channel, try_reset_channel_quota, }; -use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg}; +use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; use crate::state::{ChannelFlow, FlowType, CHANNEL_FLOWS, GOVMODULE, IBCMODULE}; // version info for migration info @@ -188,6 +188,11 @@ fn get_quotas(deps: Deps, channel_id: impl Into) -> StdResult { to_binary(&CHANNEL_FLOWS.load(deps.storage, &channel_id.into())?) } +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn migrate(_deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { + unimplemented!() +} + #[cfg(test)] mod tests { use super::*; 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 26f48cea159..9a750bfda0a 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 @@ -102,12 +102,11 @@ mod tests { funds: 300, }; let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let err = app + let _err = app .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) .unwrap_err(); // TODO: how do we check the error type here? - println!("{err:?}"); // ... Time passes app.update_block(|b| { @@ -153,9 +152,7 @@ mod tests { funds: 1, }; let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - println!("{res:?}"); + let _res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); // Another packet is rate limited let msg = ExecuteMsg::SendPacket { @@ -260,7 +257,6 @@ mod tests { b.time = b.time.plus_seconds((RESET_TIME_WEEKLY * 2) + 1) // Two weeks }); - println!("{:?}", app.block_info()); let msg = ExecuteMsg::SendPacket { channel_id: "channel".to_string(), channel_value: 100, 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 adaa323aad6..4a7fdbbf7b8 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -17,6 +17,14 @@ pub enum FlowType { Out, } +/// A Flow represents the transfer of value through an IBC channel duriong a +/// time window. +/// +/// It tracks inflows (transfers into osmosis) and outflows (transfers out of +/// osmosis). +/// +/// The period_end represents the last point in time that for which this Flow is +/// tracking the value transfer. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema, Copy)] pub struct Flow { pub inflow: u128, @@ -38,21 +46,28 @@ impl Flow { } } + /// The balance of a flow is how much absolute value has moved through the + /// channel before period_end. pub fn balance(&self) -> u128 { self.inflow.abs_diff(self.outflow) } + /// 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 } // Mutating methods + + /// Expire resets the Flow to start tracking the value transfer from the + /// moment this methos is called. pub fn expire(&mut self, now: Timestamp, duration: u64) { self.inflow = 0; self.outflow = 0; self.period_end = now.plus_seconds(duration); } + /// Updates the current flow with a transfer of value. pub fn add_flow(&mut self, direction: FlowType, value: u128) { match direction { FlowType::In => self.inflow = self.inflow.saturating_add(value), @@ -61,6 +76,13 @@ impl Flow { } } +/// A Quota is the percentage of the channel's total value that can be +/// transfered through the channel in a given period of time (duration) +/// +/// Percentages can be different for send and recv +/// +/// The name of the quota is expected to be a human-readable representation of +/// the duration (i.e.: "weekly", "daily", "every-six-months", ...) #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] pub struct Quota { pub name: String, @@ -70,7 +92,9 @@ pub struct Quota { } impl Quota { - /// Calculates the max capacity based on the total value of the channel + /// Calculates the max capacity (absolute value in the same unit as + /// total_value) in a given direction based on the total value of the + /// channel. pub fn capacity_at(&self, total_value: &u128, direction: &FlowType) -> u128 { let max_percentage = match direction { FlowType::In => self.max_percentage_recv, @@ -101,16 +125,25 @@ pub struct ChannelFlow { pub flow: Flow, } -/// Only this module can manage the contract +/// Only this address can manage the contract. This will likely be the +/// governance module, but could be set to something else if needed pub const GOVMODULE: Item = Item::new("gov_module"); -/// Only this module can execute transfers +/// Only this address can execute transfers. This will likely be the +/// IBC transfer module, but could be set to something else if needed pub const IBCMODULE: Item = Item::new("ibc_module"); -// For simplicity, the map keys (ibc channel) refers to the "host" channel on the -// osmosis side. This means that on PacketSend it will refer to the source -// channel while on PacketRecv it refers to the destination channel. -// -// It is the responsibility of the go module to pass the appropriate channel -// when sending the messages + +/// CHANNEL_FLOWS is the main state for this contract. It maps an IBC channel_id +/// to a vector of ChannelFlows. The ChannelFlow struct contains the information +/// about how much value has moved through the channel during the currently +/// active time period (channel_flow.flow) and what percentage of the channel's +/// value we are allowing to flow through that channel in a specific duration (quota) +/// +/// For simplicity, the map keys (ibc channel) refers to the "host" channel on the +/// osmosis side. This means that on PacketSend it will refer to the source +/// channel while on PacketRecv it refers to the destination channel. +/// +/// It is the responsibility of the go module to pass the appropriate channel +/// when sending the messages pub const CHANNEL_FLOWS: Map<&str, Vec> = Map::new("flow"); #[cfg(test)] From 9f605fc07955f46f39487f8c411f9c577afbad70 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Tue, 16 Aug 2022 19:08:05 +0200 Subject: [PATCH 04/29] added migration msg --- x/ibc-rate-limit/contracts/rate-limiter/src/management.rs | 2 +- x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs index b1ba33ce35b..0d29a339e3d 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs @@ -224,7 +224,7 @@ mod tests { let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); let value: Vec = from_binary(&res).unwrap(); assert_eq!(value.len(), 1); - println!("{value:?}"); + verify_query_response( &value[0], "different", diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index 35f2812db57..546aa746220 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -67,3 +67,7 @@ pub enum ExecuteMsg { pub enum QueryMsg { GetQuotas { channel_id: String }, } + +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum MigrateMsg {} From 26fc15b213a28c42c49fdde31a3d5ce1f70178ab Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 09:01:18 +0200 Subject: [PATCH 05/29] Update x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs Co-authored-by: Dev Ojha --- x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index 546aa746220..fe7a269482d 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -2,6 +2,7 @@ use cosmwasm_std::Addr; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +// The channel struct contains a name representing a unique identifier within ibc-go, and a list of rate limit quotas #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] pub struct Channel { pub name: String, From a9dd5bf33e3802d4e64fcb8e520a412694ddc3e3 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 09:11:39 +0200 Subject: [PATCH 06/29] removed gitkeep --- x/ibc-rate-limit/contracts/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 x/ibc-rate-limit/contracts/.gitkeep diff --git a/x/ibc-rate-limit/contracts/.gitkeep b/x/ibc-rate-limit/contracts/.gitkeep deleted file mode 100644 index e69de29bb2d..00000000000 From 11c5765b0c05ecbc02e8aa9ba9a1d27bc24ad8a0 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Wed, 17 Aug 2022 10:03:19 +0200 Subject: [PATCH 07/29] COmments & questions --- .../contracts/rate-limiter/src/contract.rs | 14 ++++++++++++++ .../contracts/rate-limiter/src/management.rs | 3 +++ .../contracts/rate-limiter/src/state.rs | 9 +++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 1da17deb175..1e1f9ea3045 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -88,6 +88,9 @@ pub struct ChannelFlowResponse { pub max: u128, } +// TODO: Lets add some more docs for this, namely that channel_value is the total supply of the denom +// Q: Is an ICS 20 transfer only 1 denom at a time, or does the caller have to split into several +// calls if its a multi-denom ICS-20 transfer pub fn try_transfer( deps: DepsMut, sender: Addr, @@ -98,6 +101,7 @@ pub fn try_transfer( now: Timestamp, ) -> Result { // Only the IBCMODULE can execute transfers + // TODO: Should we move this to a helper method? let ibc_module = IBCMODULE.load(deps.storage)?; if sender != ibc_module { return Err(ContractError::Unauthorized {}); @@ -113,6 +117,7 @@ pub fn try_transfer( if !configured { // No Quota configured for the current channel. Allowing all messages. + // TODO: Should there be an attribute for it being allowed vs denied? return Ok(Response::new() .add_attribute("method", "try_transfer") .add_attribute("channel_id", channel_id) @@ -124,6 +129,12 @@ pub fn try_transfer( let results: Result, _> = channels .iter_mut() .map(|channel| { + // TODO: Should we move this to more methods on ChannelFlow? + // e.g. new pseudocode + // channel.updateIfExpired(now) + // channel.trackSend(&direction, funds) + // channel.CheckRateLimit(direction.clone())?; + // (Or at least update for time + rename for track send. the last one is a bit of a code style nit) let max = channel.quota.capacity_at(&channel_value, &direction); if channel.flow.is_expired(now) { channel.flow.expire(now, channel.quota.duration) @@ -138,6 +149,7 @@ pub fn try_transfer( }); }; Ok(ChannelFlowResponse { + // TODO: nit, can we derive channel.Clone()? channel_flow: ChannelFlow { quota: channel.quota.clone(), flow: channel.flow.clone(), @@ -160,6 +172,8 @@ pub fn try_transfer( .add_attribute("channel_id", channel_id); // Adding the attributes from each quota to the response + // Code style Q: Should we move attribute setting to a function on response? + // Rust question: How does this work with one response being an error, I'm not sure how the flow works here results.iter().fold(Ok(response), |acc, result| { Ok(acc? .add_attribute( diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs index 0d29a339e3d..e5667358113 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs @@ -32,6 +32,7 @@ pub fn try_add_channel( quotas: Vec, now: Timestamp, ) -> Result { + // codenit: should we make a function for checking this authorization? let ibc_module = IBCMODULE.load(deps.storage)?; let gov_module = GOVMODULE.load(deps.storage)?; if sender != ibc_module && sender != gov_module { @@ -67,6 +68,7 @@ pub fn try_remove_channel( .add_attribute("channel_id", channel_id)) } +// Reset specified quote_id for the given channel_id pub fn try_reset_channel_quota( deps: DepsMut, sender: Addr, @@ -88,6 +90,7 @@ pub fn try_reset_channel_quota( channel_id: channel_id.clone(), }), Some(mut flows) => { + // Q: What happens here if quote_id not found? seems like we return ok? flows.iter_mut().for_each(|channel| { if channel.quota.name == channel_id.as_ref() { channel.flow.expire(now, channel.quota.duration) 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 4a7fdbbf7b8..56ae7cb3ddc 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -17,7 +17,7 @@ pub enum FlowType { Out, } -/// A Flow represents the transfer of value through an IBC channel duriong a +/// A Flow represents the transfer of value through an IBC channel during a /// time window. /// /// It tracks inflows (transfers into osmosis) and outflows (transfers out of @@ -25,8 +25,11 @@ pub enum FlowType { /// /// The period_end represents the last point in time that for which this Flow is /// tracking the value transfer. +/// TODO: Document that time windows are not rolling windows, but instead discrete repeating windows. +/// This is a design decision chosen for gas reasons. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema, Copy)] pub struct Flow { + // Q: Do we have edge case issues with inflow/outflow being u128, e.g. what if a token has super high precision. pub inflow: u128, pub outflow: u128, pub period_end: Timestamp, @@ -60,7 +63,7 @@ impl Flow { // Mutating methods /// Expire resets the Flow to start tracking the value transfer from the - /// moment this methos is called. + /// moment this method is called. pub fn expire(&mut self, now: Timestamp, duration: u64) { self.inflow = 0; self.outflow = 0; @@ -119,6 +122,8 @@ impl From<&QuotaMsg> for Quota { } } +// TODO: Add docs that this is the main tracker, we should be setting multiple of these per contract +// Q: Should we rename to "ChannelRateLimiter", and rename flow to "FlowTracker"? #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] pub struct ChannelFlow { pub quota: Quota, From 73acb2fc74d4d7ec80e015b3902d2cb66e69f101 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 11:32:00 +0200 Subject: [PATCH 08/29] workflow without test data checks and clippy fixes --- .github/workflows/contracts.yml | 65 +++++++++++++++---- .../contracts/rate-limiter/src/contract.rs | 4 +- .../contracts/rate-limiter/src/helpers.rs | 2 +- .../contracts/rate-limiter/src/msg.rs | 12 ++-- .../contracts/rate-limiter/src/state.rs | 6 +- 5 files changed, 65 insertions(+), 24 deletions(-) diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml index 29786ea7317..6ddb63b5b15 100644 --- a/.github/workflows/contracts.yml +++ b/.github/workflows/contracts.yml @@ -10,8 +10,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - workdir: [./x/ibc-rate-limit] -# output: [./x/ibc-rate-limit/testdata/rate_limit.wasm] + contract: [{workdir: ./x/ibc-rate-limit/, output: testdata/rate_limiter.wasm, build: artifacts/rate_limiter-x86_64.wasm, name: rate_limiter}] steps: - name: Checkout sources @@ -23,24 +22,59 @@ jobs: toolchain: nightly target: wasm32-unknown-unknown - - name: Build - working-directory: ${{ matrix.workdir }} + - name: Add the wasm target + working-directory: ${{ matrix.contract.workdir }} run: > rustup target add wasm32-unknown-unknown; + + + - name: Build + working-directory: ${{ matrix.contract.workdir }} + run: > cargo build --release --target wasm32-unknown-unknown - name: Test - working-directory: ${{ matrix.workdir }} + working-directory: ${{ matrix.contract.workdir }} run: > cargo test + - name: Set latest cw-optimizoor version + run: > + echo "CW_OPTIMIZOOR_VERSION=`cargo search cw-optimizoor -q | cut -d '"' -f 2`" >> $GITHUB_ENV + + - name: Cache cw-optimizoor + id: cache-cw-optimizoor + uses: actions/cache@v3 + env: + cache-name: cache-cw-optimizoor + with: + # cargo bin files are stored in `~/.cargo/bin/` on Linux/macOS + path: ~/.cargo/bin/cargo-cw-optimizoor + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ env.CW_OPTIMIZOOR_VERSION }} + + - if: ${{ steps.cache-cw-optimizoor.outputs.cache-hit != 'true' }} + name: Install cw-optimizoor + continue-on-error: true + run: > + cargo install cw-optimizoor + + - name: Optimize + working-directory: ${{ matrix.contract.workdir }} + run: > + cargo cw-optimizoor + + - name: 'Upload optimized contract artifact' + uses: actions/upload-artifact@v3 + with: + name: ${{ matrix.contract.name }} + path: ${{ matrix.contract.workdir }}${{ matrix.contract.build }} + retention-days: 1 + # - name: Check Test Data -# working-directory: ${{ matrix.workdir }} -# if: ${{ matrix.output != null }} +# working-directory: ${{ matrix.contract.workdir }} +# if: ${{ matrix.contract.output != null }} # run: > -# ls ${{ matrix.output }}; -# ls ${{ matrix.output }}/artifacts; -# diff ${{ matrix.output }} ./artifacts/*.wasm +# diff ${{ matrix.contract.output }} ${{ matrix.contract.build }} lints: @@ -57,11 +91,18 @@ jobs: - name: Install toolchain uses: actions-rs/toolchain@v1 with: + profile: minimal toolchain: nightly - components: rustfmt - target: wasm32-unknown-unknown + override: true + components: rustfmt, clippy - name: Format working-directory: ${{ matrix.workdir }} run: > cargo fmt --all -- --check + + - name: run cargo clippy + working-directory: ${{ matrix.workdir }} + run: > + cargo clippy -- -D warnings + diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 1e1f9ea3045..f1bae04c530 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -111,7 +111,7 @@ pub fn try_transfer( let configured = match channels { None => false, - Some(ref x) if x.len() == 0 => false, + Some(ref x) if x.is_empty() => false, _ => true, }; @@ -152,7 +152,7 @@ pub fn try_transfer( // TODO: nit, can we derive channel.Clone()? channel_flow: ChannelFlow { quota: channel.quota.clone(), - flow: channel.flow.clone(), + flow: channel.flow, }, used: balance, max, diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs index 82f4f1168ba..d7e86c98656 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs @@ -7,7 +7,7 @@ use crate::msg::ExecuteMsg; /// CwTemplateContract is a wrapper around Addr that provides a lot of helpers /// for working with this. -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct RateLimitingContract(pub Addr); impl RateLimitingContract { diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index fe7a269482d..dcf3be447ef 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -3,13 +3,13 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; // The channel struct contains a name representing a unique identifier within ibc-go, and a list of rate limit quotas -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct Channel { pub name: String, pub quotas: Vec, } -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct QuotaMsg { pub name: String, pub duration: u64, @@ -28,7 +28,7 @@ impl QuotaMsg { /// Initialize the contract with the address of the IBC module and any existing channels. /// Only the ibc module is allowed to execute actions on this contract -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct InstantiateMsg { pub gov_module: Addr, pub ibc_module: Addr, @@ -37,7 +37,7 @@ pub struct InstantiateMsg { /// The caller (IBC module) is responsibble for correctly calculating the funds /// being sent through the channel -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum ExecuteMsg { SendPacket { @@ -63,12 +63,12 @@ pub enum ExecuteMsg { }, } -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum QueryMsg { GetQuotas { channel_id: String }, } -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum MigrateMsg {} 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 56ae7cb3ddc..88c8a5adaab 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -27,7 +27,7 @@ pub enum FlowType { /// tracking the value transfer. /// TODO: Document that time windows are not rolling windows, but instead discrete repeating windows. /// This is a design decision chosen for gas reasons. -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema, Copy)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema, Copy)] pub struct Flow { // Q: Do we have edge case issues with inflow/outflow being u128, e.g. what if a token has super high precision. pub inflow: u128, @@ -86,7 +86,7 @@ impl Flow { /// /// The name of the quota is expected to be a human-readable representation of /// the duration (i.e.: "weekly", "daily", "every-six-months", ...) -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct Quota { pub name: String, pub max_percentage_send: u32, @@ -124,7 +124,7 @@ impl From<&QuotaMsg> for Quota { // TODO: Add docs that this is the main tracker, we should be setting multiple of these per contract // Q: Should we rename to "ChannelRateLimiter", and rename flow to "FlowTracker"? -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct ChannelFlow { pub quota: Quota, pub flow: Flow, From 48b19cb59a0305f6990833addfe712b437047f0c Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 13:18:03 +0200 Subject: [PATCH 09/29] renames and code fixes --- .../contracts/rate-limiter/src/contract.rs | 67 ++++++++++--------- .../contracts/rate-limiter/src/helpers.rs | 4 +- .../contracts/rate-limiter/src/management.rs | 30 ++++----- .../contracts/rate-limiter/src/state.rs | 13 ++-- 4 files changed, 61 insertions(+), 53 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index f1bae04c530..c2475b17656 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -10,7 +10,7 @@ use crate::management::{ add_new_channels, try_add_channel, try_remove_channel, try_reset_channel_quota, }; use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{ChannelFlow, FlowType, CHANNEL_FLOWS, GOVMODULE, IBCMODULE}; +use crate::state::{RateLimit, FlowType, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:rate-limiter"; @@ -82,15 +82,21 @@ pub fn execute( } } -pub struct ChannelFlowResponse { - pub channel_flow: ChannelFlow, +pub struct RateLimitResponse { + pub rate_limit: RateLimit, pub used: u128, pub max: u128, } -// TODO: Lets add some more docs for this, namely that channel_value is the total supply of the denom // Q: Is an ICS 20 transfer only 1 denom at a time, or does the caller have to split into several // calls if its a multi-denom ICS-20 transfer + +/// This function checks the rate limit and, if successful, stores the updated data about the value +/// that has been transfered through the channel. +/// If the period for a RateLimit has ended, the Flow information is reset. +/// +/// The channel_value is the current value of the channel as calculated by the caller. This should +/// be the total supply of a denom pub fn try_transfer( deps: DepsMut, sender: Addr, @@ -102,14 +108,16 @@ pub fn try_transfer( ) -> Result { // Only the IBCMODULE can execute transfers // TODO: Should we move this to a helper method? + // This may not be needed once we move this function to be under sudo. + // Though it might still be worth checking that only the transfer module is calling it let ibc_module = IBCMODULE.load(deps.storage)?; if sender != ibc_module { return Err(ContractError::Unauthorized {}); } - let channels = CHANNEL_FLOWS.may_load(deps.storage, &channel_id)?; + let trackers = RATE_LIMIT_TRACKERS.may_load(deps.storage, &channel_id)?; - let configured = match channels { + let configured = match trackers { None => false, Some(ref x) if x.is_empty() => false, _ => true, @@ -124,35 +132,35 @@ pub fn try_transfer( .add_attribute("quota", "none")); } - let mut channels = channels.unwrap(); + let mut rate_limits = trackers.unwrap(); - let results: Result, _> = channels + let results: Result, _> = rate_limits .iter_mut() - .map(|channel| { + .map(|limit| { // TODO: Should we move this to more methods on ChannelFlow? // e.g. new pseudocode // channel.updateIfExpired(now) // channel.trackSend(&direction, funds) // channel.CheckRateLimit(direction.clone())?; // (Or at least update for time + rename for track send. the last one is a bit of a code style nit) - let max = channel.quota.capacity_at(&channel_value, &direction); - if channel.flow.is_expired(now) { - channel.flow.expire(now, channel.quota.duration) + let max = limit.quota.capacity_at(&channel_value, &direction); + if limit.flow.is_expired(now) { + limit.flow.expire(now, limit.quota.duration) } - channel.flow.add_flow(direction.clone(), funds); + limit.flow.add_flow(direction.clone(), funds); - let balance = channel.flow.balance(); + let balance = limit.flow.balance(); if balance > max { return Err(ContractError::RateLimitExceded { channel: channel_id.to_string(), - reset: channel.flow.period_end, + reset: limit.flow.period_end, }); }; - Ok(ChannelFlowResponse { + Ok(RateLimitResponse { // TODO: nit, can we derive channel.Clone()? - channel_flow: ChannelFlow { - quota: channel.quota.clone(), - flow: channel.flow, + rate_limit: RateLimit { + quota: limit.quota.clone(), + flow: limit.flow, }, used: balance, max, @@ -161,10 +169,10 @@ pub fn try_transfer( .collect(); let results = results?; - CHANNEL_FLOWS.save( + RATE_LIMIT_TRACKERS.save( deps.storage, &channel_id, - &results.iter().map(|r| r.channel_flow.clone()).collect(), + &results.iter().map(|r| r.rate_limit.clone()).collect(), )?; let response = Response::new() @@ -177,16 +185,16 @@ pub fn try_transfer( results.iter().fold(Ok(response), |acc, result| { Ok(acc? .add_attribute( - format!("{}_used", result.channel_flow.quota.name), + format!("{}_used", result.rate_limit.quota.name), result.used.to_string(), ) .add_attribute( - format!("{}_max", result.channel_flow.quota.name), + format!("{}_max", result.rate_limit.quota.name), result.max.to_string(), ) .add_attribute( - format!("{}_period_end", result.channel_flow.quota.name), - result.channel_flow.flow.period_end.to_string(), + format!("{}_period_end", result.rate_limit.quota.name), + result.rate_limit.flow.period_end.to_string(), )) }) } @@ -199,7 +207,7 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } fn get_quotas(deps: Deps, channel_id: impl Into) -> StdResult { - to_binary(&CHANNEL_FLOWS.load(deps.storage, &channel_id.into())?) + to_binary(&RATE_LIMIT_TRACKERS.load(deps.storage, &channel_id.into())?) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -361,7 +369,6 @@ mod tests { let err = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap_err(); assert!(matches!(err, ContractError::RateLimitExceded { .. })); - //assert_eq!(18, value.count); } #[test] @@ -438,7 +445,7 @@ mod tests { }; let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); - let value: Vec = from_binary(&res).unwrap(); + let value: Vec = from_binary(&res).unwrap(); assert_eq!(value[0].quota.name, "weekly"); assert_eq!(value[0].quota.max_percentage_send, 10); assert_eq!(value[0].quota.max_percentage_recv, 10); @@ -467,7 +474,7 @@ mod tests { // Query let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); - let value: Vec = from_binary(&res).unwrap(); + let value: Vec = from_binary(&res).unwrap(); verify_query_response( &value[0], "weekly", @@ -506,7 +513,7 @@ mod tests { channel_id: "channel".to_string(), }; let res = query(deps.as_ref(), env.clone(), query_msg).unwrap(); - let value: Vec = from_binary(&res).unwrap(); + let value: Vec = from_binary(&res).unwrap(); verify_query_response( &value[0], "bad_quota", diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs index d7e86c98656..092b24508cd 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs @@ -30,10 +30,10 @@ impl RateLimitingContract { pub mod tests { use cosmwasm_std::Timestamp; - use crate::state::ChannelFlow; + use crate::state::RateLimit; pub fn verify_query_response( - value: &ChannelFlow, + value: &RateLimit, quota_name: &str, send_recv: (u32, u32), duration: u64, diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs index e5667358113..fe4a38969ca 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs @@ -1,5 +1,5 @@ use crate::msg::{Channel, QuotaMsg}; -use crate::state::{ChannelFlow, Flow, CHANNEL_FLOWS, GOVMODULE, IBCMODULE}; +use crate::state::{RateLimit, Flow, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; use crate::ContractError; use cosmwasm_std::{Addr, DepsMut, Response, Timestamp}; @@ -9,13 +9,13 @@ pub fn add_new_channels( now: Timestamp, ) -> Result<(), ContractError> { for channel in channels { - CHANNEL_FLOWS.save( + RATE_LIMIT_TRACKERS.save( deps.storage, &channel.name, &channel .quotas .iter() - .map(|q| ChannelFlow { + .map(|q| RateLimit { quota: q.into(), flow: Flow::new(0_u128, 0_u128, now, q.duration), }) @@ -62,7 +62,7 @@ pub fn try_remove_channel( if sender != ibc_module && sender != gov_module { return Err(ContractError::Unauthorized {}); } - CHANNEL_FLOWS.remove(deps.storage, &channel_id); + RATE_LIMIT_TRACKERS.remove(deps.storage, &channel_id); Ok(Response::new() .add_attribute("method", "try_remove_channel") .add_attribute("channel_id", channel_id)) @@ -81,22 +81,22 @@ pub fn try_reset_channel_quota( return Err(ContractError::Unauthorized {}); } - CHANNEL_FLOWS.update( + RATE_LIMIT_TRACKERS.update( deps.storage, &channel_id.clone(), - |maybe_flows| match maybe_flows { + |maybe_rate_limit| match maybe_rate_limit { None => Err(ContractError::QuotaNotFound { quota_id, channel_id: channel_id.clone(), }), - Some(mut flows) => { + Some(mut limits) => { // Q: What happens here if quote_id not found? seems like we return ok? - flows.iter_mut().for_each(|channel| { - if channel.quota.name == channel_id.as_ref() { - channel.flow.expire(now, channel.quota.duration) + limits.iter_mut().for_each(|limit| { + if limit.quota.name == channel_id.as_ref() { + limit.flow.expire(now, limit.quota.duration) } }); - Ok(flows) + Ok(limits) } }, )?; @@ -115,7 +115,7 @@ mod tests { use crate::contract::{execute, query}; use crate::helpers::tests::verify_query_response; use crate::msg::{ExecuteMsg, QueryMsg, QuotaMsg}; - use crate::state::{ChannelFlow, GOVMODULE, IBCMODULE}; + use crate::state::{RateLimit, GOVMODULE, IBCMODULE}; const IBC_ADDR: &str = "IBC_MODULE"; const GOV_ADDR: &str = "GOV_MODULE"; @@ -150,7 +150,7 @@ mod tests { let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); - let value: Vec = from_binary(&res).unwrap(); + let value: Vec = from_binary(&res).unwrap(); verify_query_response( &value[0], "daily", @@ -195,7 +195,7 @@ mod tests { channel_id: "channel2".to_string(), }; let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); - let value: Vec = from_binary(&res).unwrap(); + let value: Vec = from_binary(&res).unwrap(); assert_eq!(value.len(), 1); verify_query_response( &value[0], @@ -225,7 +225,7 @@ mod tests { channel_id: "channel2".to_string(), }; let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); - let value: Vec = from_binary(&res).unwrap(); + let value: Vec = from_binary(&res).unwrap(); assert_eq!(value.len(), 1); verify_query_response( 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 88c8a5adaab..bd6dd20b53a 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -122,10 +122,11 @@ impl From<&QuotaMsg> for Quota { } } -// TODO: Add docs that this is the main tracker, we should be setting multiple of these per contract -// Q: Should we rename to "ChannelRateLimiter", and rename flow to "FlowTracker"? +/// RateLimit is the main structure tracked for each channel. Its quota +/// represents rate limit configuration, and the flow its +/// current state (i.e.: how much value has been transfered in the current period) #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] -pub struct ChannelFlow { +pub struct RateLimit { pub quota: Quota, pub flow: Flow, } @@ -137,8 +138,8 @@ pub const GOVMODULE: Item = Item::new("gov_module"); /// IBC transfer module, but could be set to something else if needed pub const IBCMODULE: Item = Item::new("ibc_module"); -/// CHANNEL_FLOWS is the main state for this contract. It maps an IBC channel_id -/// to a vector of ChannelFlows. The ChannelFlow struct contains the information +/// RATE_LIMIT_TRACKERS is the main state for this contract. It maps an IBC channel_id +/// to a vector of `RateLimit`s. The `RateLimit` struct contains the information /// about how much value has moved through the channel during the currently /// active time period (channel_flow.flow) and what percentage of the channel's /// value we are allowing to flow through that channel in a specific duration (quota) @@ -149,7 +150,7 @@ pub const IBCMODULE: Item = Item::new("ibc_module"); /// /// It is the responsibility of the go module to pass the appropriate channel /// when sending the messages -pub const CHANNEL_FLOWS: Map<&str, Vec> = Map::new("flow"); +pub const RATE_LIMIT_TRACKERS: Map<&str, Vec> = Map::new("flow"); #[cfg(test)] mod tests { From 6180eb847a9b07ce414e4558965c0ab2dd047936 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 13:28:09 +0200 Subject: [PATCH 10/29] reordered imports --- x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index c2475b17656..3f780422ce2 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -10,7 +10,7 @@ use crate::management::{ add_new_channels, try_add_channel, try_remove_channel, try_reset_channel_quota, }; use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{RateLimit, FlowType, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::state::{FlowType, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:rate-limiter"; From 82c83b1139a4cab22df72150170f597d5d730cff Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 13:34:30 +0200 Subject: [PATCH 11/29] cargo fmt --- .../contracts/rate-limiter/src/management.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs index fe4a38969ca..289335ccb60 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs @@ -1,5 +1,5 @@ use crate::msg::{Channel, QuotaMsg}; -use crate::state::{RateLimit, Flow, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::state::{Flow, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; use crate::ContractError; use cosmwasm_std::{Addr, DepsMut, Response, Timestamp}; @@ -81,10 +81,8 @@ pub fn try_reset_channel_quota( return Err(ContractError::Unauthorized {}); } - RATE_LIMIT_TRACKERS.update( - deps.storage, - &channel_id.clone(), - |maybe_rate_limit| match maybe_rate_limit { + RATE_LIMIT_TRACKERS.update(deps.storage, &channel_id.clone(), |maybe_rate_limit| { + match maybe_rate_limit { None => Err(ContractError::QuotaNotFound { quota_id, channel_id: channel_id.clone(), @@ -98,8 +96,8 @@ pub fn try_reset_channel_quota( }); Ok(limits) } - }, - )?; + } + })?; Ok(Response::new() .add_attribute("method", "try_reset_channel") From d4ace7b8989eab74278441cb854cd6653858d46d Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 19:13:30 +0200 Subject: [PATCH 12/29] added danom tracking --- .../contracts/rate-limiter/src/contract.rs | 160 +++++++++++------- .../contracts/rate-limiter/src/error.rs | 9 +- .../rate-limiter/src/integration_tests.rs | 61 ++++--- .../contracts/rate-limiter/src/management.rs | 81 +++++---- .../contracts/rate-limiter/src/msg.rs | 35 +++- .../contracts/rate-limiter/src/state.rs | 74 +++++--- 6 files changed, 273 insertions(+), 147 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 3f780422ce2..b85d9028d1f 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -6,11 +6,9 @@ use cosmwasm_std::{ use cw2::set_contract_version; use crate::error::ContractError; -use crate::management::{ - add_new_channels, try_add_channel, try_remove_channel, try_reset_channel_quota, -}; +use crate::management::{add_new_paths, try_add_path, try_remove_path, try_reset_path_quota}; use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{FlowType, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::state::{FlowType, Path, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:rate-limiter"; @@ -27,7 +25,7 @@ pub fn instantiate( IBCMODULE.save(deps.storage, &msg.ibc_module)?; GOVMODULE.save(deps.storage, &msg.gov_module)?; - add_new_channels(deps, msg.channels, env.block.time)?; + add_new_paths(deps, msg.paths, env.block.time)?; Ok(Response::new() .add_attribute("method", "instantiate") @@ -47,10 +45,12 @@ pub fn execute( channel_id, channel_value, funds, + denom, } => try_transfer( deps, info.sender, channel_id, + denom, channel_value, funds, FlowType::Out, @@ -60,25 +60,37 @@ pub fn execute( channel_id, channel_value, funds, + denom, } => try_transfer( deps, info.sender, channel_id, + denom, channel_value, funds, FlowType::In, env.block.time, ), - ExecuteMsg::AddChannel { channel_id, quotas } => { - try_add_channel(deps, info.sender, channel_id, quotas, env.block.time) - } - ExecuteMsg::RemoveChannel { channel_id } => { - try_remove_channel(deps, info.sender, channel_id) + ExecuteMsg::AddPath { + channel_id, + denom, + quotas, + } => try_add_path(deps, info.sender, channel_id, denom, quotas, env.block.time), + ExecuteMsg::RemovePath { channel_id, denom } => { + try_remove_path(deps, info.sender, channel_id, denom) } - ExecuteMsg::ResetChannelQuota { + ExecuteMsg::ResetPathQuota { + channel_id, + denom, + quota_id, + } => try_reset_path_quota( + deps, + info.sender, channel_id, + denom, quota_id, - } => try_reset_channel_quota(deps, info.sender, channel_id, quota_id, env.block.time), + env.block.time, + ), } } @@ -92,15 +104,16 @@ pub struct RateLimitResponse { // calls if its a multi-denom ICS-20 transfer /// This function checks the rate limit and, if successful, stores the updated data about the value -/// that has been transfered through the channel. +/// that has been transfered through the channel for a specific denom. /// If the period for a RateLimit has ended, the Flow information is reset. /// -/// The channel_value is the current value of the channel as calculated by the caller. This should -/// be the total supply of a denom +/// The channel_value is the current value of the denom for the the channel as +/// calculated by the caller. This should be the total supply of a denom pub fn try_transfer( deps: DepsMut, sender: Addr, channel_id: String, + denom: String, channel_value: u128, funds: u128, direction: FlowType, @@ -115,7 +128,8 @@ pub fn try_transfer( return Err(ContractError::Unauthorized {}); } - let trackers = RATE_LIMIT_TRACKERS.may_load(deps.storage, &channel_id)?; + let path = Path::new(&channel_id, &denom); + let trackers = RATE_LIMIT_TRACKERS.may_load(deps.storage, path.into())?; let configured = match trackers { None => false, @@ -124,11 +138,12 @@ pub fn try_transfer( }; if !configured { - // No Quota configured for the current channel. Allowing all messages. + // No Quota configured for the current path. Allowing all messages. // TODO: Should there be an attribute for it being allowed vs denied? return Ok(Response::new() .add_attribute("method", "try_transfer") .add_attribute("channel_id", channel_id) + .add_attribute("denom", denom) .add_attribute("quota", "none")); } @@ -153,6 +168,7 @@ pub fn try_transfer( if balance > max { return Err(ContractError::RateLimitExceded { channel: channel_id.to_string(), + denom: denom.to_string(), reset: limit.flow.period_end, }); }; @@ -171,13 +187,14 @@ pub fn try_transfer( RATE_LIMIT_TRACKERS.save( deps.storage, - &channel_id, + Path::new(&channel_id, &denom).into(), &results.iter().map(|r| r.rate_limit.clone()).collect(), )?; let response = Response::new() .add_attribute("method", "try_transfer") - .add_attribute("channel_id", channel_id); + .add_attribute("channel_id", channel_id) + .add_attribute("denom", denom); // Adding the attributes from each quota to the response // Code style Q: Should we move attribute setting to a function on response? @@ -202,12 +219,17 @@ pub fn try_transfer( #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { - QueryMsg::GetQuotas { channel_id } => get_quotas(deps, channel_id), + QueryMsg::GetQuotas { channel_id, denom } => get_quotas(deps, channel_id, denom), } } -fn get_quotas(deps: Deps, channel_id: impl Into) -> StdResult { - to_binary(&RATE_LIMIT_TRACKERS.load(deps.storage, &channel_id.into())?) +fn get_quotas( + deps: Deps, + channel_id: impl Into, + denom: impl Into, +) -> StdResult { + let path = Path::new(channel_id, denom); + to_binary(&RATE_LIMIT_TRACKERS.load(deps.storage, path.into())?) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -222,7 +244,7 @@ mod tests { use cosmwasm_std::{from_binary, Addr, Attribute}; use crate::helpers::tests::verify_query_response; - use crate::msg::{Channel, QuotaMsg}; + use crate::msg::{PathMsg, QuotaMsg}; use crate::state::RESET_TIME_WEEKLY; const IBC_ADDR: &str = "IBC_MODULE"; @@ -235,7 +257,7 @@ mod tests { let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels: vec![], + paths: vec![], }; let info = mock_info(IBC_ADDR, &vec![]); @@ -256,8 +278,9 @@ mod tests { let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels: vec![Channel { - name: "channel".to_string(), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![quota], }], }; @@ -265,7 +288,8 @@ mod tests { instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; @@ -276,7 +300,8 @@ mod tests { let info = mock_info("SomeoneElse", &vec![]); let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; @@ -292,8 +317,9 @@ mod tests { let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels: vec![Channel { - name: "channel".to_string(), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![quota], }], }; @@ -301,18 +327,21 @@ mod tests { let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; let info = mock_info(IBC_ADDR, &vec![]); let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - let Attribute { key, value } = &res.attributes[2]; + + let Attribute { key, value } = &res.attributes[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "300"); let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; @@ -328,8 +357,9 @@ mod tests { let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels: vec![Channel { - name: "channel".to_string(), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![quota], }], }; @@ -338,31 +368,33 @@ mod tests { let info = mock_info(IBC_ADDR, &vec![]); let send_msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; let recv_msg = ExecuteMsg::RecvPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; let res = execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); - let Attribute { key, value } = &res.attributes[2]; + let Attribute { key, value } = &res.attributes[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "300"); let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); - let Attribute { key, value } = &res.attributes[2]; + let Attribute { key, value } = &res.attributes[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "0"); - // We can still use the channel. Even if we have sent more than the - // allowance through the channel (900 > 3000*.1), the current "balance" - // of inflow vs outflow is still lower than the channel's capacity/quota + // We can still use the path. Even if we have sent more than the + // allowance through the path (900 > 3000*.1), the current "balance" + // of inflow vs outflow is still lower than the path's capacity/quota let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); - let Attribute { key, value } = &res.attributes[2]; + let Attribute { key, value } = &res.attributes[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "300"); @@ -379,8 +411,9 @@ mod tests { let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels: vec![Channel { - name: "channel".to_string(), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![quota], }], }; @@ -389,32 +422,35 @@ mod tests { // Sending 2% let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 60, }; let info = mock_info(IBC_ADDR, &vec![]); let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - let Attribute { key, value } = &res.attributes[2]; + let Attribute { key, value } = &res.attributes[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "60"); // Sending 1% more. Allowed, as sending has a 10% allowance let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 30, }; let info = mock_info(IBC_ADDR, &vec![]); let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - let Attribute { key, value } = &res.attributes[2]; + let Attribute { key, value } = &res.attributes[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "90"); - // Receiving 1% should fail. 3% already executed through the channel + // Receiving 1% should fail. 3% already executed through the path let recv_msg = ExecuteMsg::RecvPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 30, }; @@ -431,8 +467,9 @@ mod tests { let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels: vec![Channel { - name: "channel".to_string(), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![quota], }], }; @@ -441,7 +478,8 @@ mod tests { let _res = instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); let query_msg = QueryMsg::GetQuotas { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), }; let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); @@ -459,14 +497,16 @@ mod tests { let info = mock_info(IBC_ADDR, &vec![]); let send_msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); let recv_msg = ExecuteMsg::RecvPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 30, }; @@ -493,8 +533,9 @@ mod tests { let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels: vec![Channel { - name: "channel".to_string(), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![QuotaMsg { name: "bad_quota".to_string(), duration: 200, @@ -510,7 +551,8 @@ mod tests { // If a quota is higher than 100%, we set it to 100% let query_msg = QueryMsg::GetQuotas { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), }; let res = query(deps.as_ref(), env.clone(), query_msg).unwrap(); let value: Vec = from_binary(&res).unwrap(); 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 3c58ce21bc9..dc40f708d1c 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/error.rs @@ -9,12 +9,17 @@ pub enum ContractError { #[error("Unauthorized")] Unauthorized {}, - #[error("IBC Rate Limit exceded for channel {channel:?}. Try again after {reset:?}")] - RateLimitExceded { channel: String, reset: Timestamp }, + #[error("IBC Rate Limit exceded for channel {channel:?} and denom {denom:?}. Try again after {reset:?}")] + RateLimitExceded { + channel: String, + denom: String, + reset: Timestamp, + }, #[error("Quota {quota_id} not found for channel {channel_id}")] QuotaNotFound { quota_id: String, channel_id: String, + denom: String, }, } 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 9a750bfda0a..ba6c189ee86 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 @@ -1,7 +1,7 @@ #[cfg(test)] mod tests { - use crate::helpers::RateLimitingContract; - use crate::msg::{Channel, InstantiateMsg}; + use crate::msg::InstantiateMsg; + use crate::{helpers::RateLimitingContract, msg::PathMsg}; use cosmwasm_std::{Addr, Coin, Empty, Uint128}; use cw_multi_test::{App, AppBuilder, Contract, ContractWrapper, Executor}; @@ -35,14 +35,14 @@ mod tests { }) } - fn proper_instantiate(channels: Vec) -> (App, RateLimitingContract) { + fn proper_instantiate(paths: Vec) -> (App, RateLimitingContract) { let mut app = mock_app(); let cw_template_id = app.store_code(contract_template()); let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), - channels, + paths, }; let cw_template_contract_addr = app @@ -66,7 +66,7 @@ mod tests { use super::*; use crate::{ - msg::{Channel, ExecuteMsg, QuotaMsg}, + msg::{ExecuteMsg, PathMsg, QuotaMsg}, state::{RESET_TIME_DAILY, RESET_TIME_MONTHLY, RESET_TIME_WEEKLY}, }; @@ -74,30 +74,33 @@ mod tests { fn expiration() { let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); - let (mut app, cw_template_contract) = proper_instantiate(vec![Channel { - name: "channel".to_string(), + let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![quota], }]); // Using all the allowance let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; let cosmos_msg = cw_template_contract.call(msg).unwrap(); let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - let Attribute { key, value } = &res.custom_attrs(1)[2]; + let Attribute { key, value } = &res.custom_attrs(1)[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[3]; + let Attribute { key, value } = &res.custom_attrs(1)[4]; assert_eq!(key, "weekly_max"); assert_eq!(value, "300"); // Another packet is rate limited let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; @@ -116,7 +119,8 @@ mod tests { // Sending the packet should work now let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 3_000, funds: 300, }; @@ -124,10 +128,10 @@ mod tests { let cosmos_msg = cw_template_contract.call(msg).unwrap(); let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - let Attribute { key, value } = &res.custom_attrs(1)[2]; + let Attribute { key, value } = &res.custom_attrs(1)[3]; assert_eq!(key, "weekly_used"); assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[3]; + let Attribute { key, value } = &res.custom_attrs(1)[4]; assert_eq!(key, "weekly_max"); assert_eq!(value, "300"); } @@ -140,14 +144,16 @@ mod tests { QuotaMsg::new("monthly", RESET_TIME_MONTHLY, 5, 5), ]; - let (mut app, cw_template_contract) = proper_instantiate(vec![Channel { - name: "channel".to_string(), + let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), quotas, }]); // Sending 1% to use the daily allowance let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; @@ -156,7 +162,8 @@ mod tests { // Another packet is rate limited let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; @@ -173,7 +180,8 @@ mod tests { // Sending the packet should work now let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; @@ -191,7 +199,8 @@ mod tests { // Sending the packet should work now let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; @@ -207,7 +216,8 @@ mod tests { // We now have exceeded the weekly limit! Even if the daily limit allows us, the weekly doesn't let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; @@ -224,7 +234,8 @@ mod tests { // We can still can't send because the weekly and monthly limits are the same let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; @@ -242,7 +253,8 @@ mod tests { // We can still can't send because the monthly limit hasn't passed let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; @@ -258,7 +270,8 @@ mod tests { }); let msg = ExecuteMsg::SendPacket { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), channel_value: 100, funds: 1, }; diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs index 289335ccb60..e54377f02af 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs @@ -1,18 +1,20 @@ -use crate::msg::{Channel, QuotaMsg}; -use crate::state::{Flow, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::msg::{PathMsg, QuotaMsg}; +use crate::state::{Flow, Path, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; use crate::ContractError; use cosmwasm_std::{Addr, DepsMut, Response, Timestamp}; -pub fn add_new_channels( +pub fn add_new_paths( deps: DepsMut, - channels: Vec, + path_msgs: Vec, now: Timestamp, ) -> Result<(), ContractError> { - for channel in channels { + for path_msg in path_msgs { + let path = Path::new(path_msg.channel_id, path_msg.denom); + RATE_LIMIT_TRACKERS.save( deps.storage, - &channel.name, - &channel + path.into(), + &path_msg .quotas .iter() .map(|q| RateLimit { @@ -25,10 +27,11 @@ pub fn add_new_channels( Ok(()) } -pub fn try_add_channel( +pub fn try_add_path( deps: DepsMut, sender: Addr, channel_id: String, + denom: String, quotas: Vec, now: Timestamp, ) -> Result { @@ -38,41 +41,40 @@ pub fn try_add_channel( if sender != ibc_module && sender != gov_module { return Err(ContractError::Unauthorized {}); } - add_new_channels( - deps, - vec![Channel { - name: channel_id.to_string(), - quotas, - }], - now, - )?; + add_new_paths(deps, vec![PathMsg::new(&channel_id, &denom, quotas)], now)?; Ok(Response::new() .add_attribute("method", "try_add_channel") - .add_attribute("channel_id", channel_id)) + .add_attribute("channel_id", channel_id) + .add_attribute("denom", denom)) } -pub fn try_remove_channel( +pub fn try_remove_path( deps: DepsMut, sender: Addr, channel_id: String, + denom: String, ) -> Result { let ibc_module = IBCMODULE.load(deps.storage)?; let gov_module = GOVMODULE.load(deps.storage)?; if sender != ibc_module && sender != gov_module { return Err(ContractError::Unauthorized {}); } - RATE_LIMIT_TRACKERS.remove(deps.storage, &channel_id); + + let path = Path::new(&channel_id, &denom); + RATE_LIMIT_TRACKERS.remove(deps.storage, path.into()); Ok(Response::new() .add_attribute("method", "try_remove_channel") + .add_attribute("denom", denom) .add_attribute("channel_id", channel_id)) } // Reset specified quote_id for the given channel_id -pub fn try_reset_channel_quota( +pub fn try_reset_path_quota( deps: DepsMut, sender: Addr, channel_id: String, + denom: String, quota_id: String, now: Timestamp, ) -> Result { @@ -81,16 +83,18 @@ pub fn try_reset_channel_quota( return Err(ContractError::Unauthorized {}); } - RATE_LIMIT_TRACKERS.update(deps.storage, &channel_id.clone(), |maybe_rate_limit| { + let path = Path::new(&channel_id, &denom); + RATE_LIMIT_TRACKERS.update(deps.storage, path.into(), |maybe_rate_limit| { match maybe_rate_limit { None => Err(ContractError::QuotaNotFound { quota_id, channel_id: channel_id.clone(), + denom: denom.clone(), }), Some(mut limits) => { // Q: What happens here if quote_id not found? seems like we return ok? limits.iter_mut().for_each(|limit| { - if limit.quota.name == channel_id.as_ref() { + if limit.quota.name == quota_id.as_ref() { limit.flow.expire(now, limit.quota.duration) } }); @@ -128,8 +132,9 @@ mod tests { .save(deps.as_mut().storage, &Addr::unchecked(GOV_ADDR)) .unwrap(); - let msg = ExecuteMsg::AddChannel { - channel_id: "channel".to_string(), + let msg = ExecuteMsg::AddPath { + channel_id: format!("channel"), + denom: format!("denom"), quotas: vec![QuotaMsg { name: "daily".to_string(), duration: 1600, @@ -143,7 +148,8 @@ mod tests { assert_eq!(0, res.messages.len()); let query_msg = QueryMsg::GetQuotas { - channel_id: "channel".to_string(), + channel_id: format!("channel"), + denom: format!("denom"), }; let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); @@ -161,9 +167,10 @@ mod tests { assert_eq!(value.len(), 1); - // Add another channel - let msg = ExecuteMsg::AddChannel { - channel_id: "channel2".to_string(), + // Add another path + let msg = ExecuteMsg::AddPath { + channel_id: format!("channel2"), + denom: format!("denom"), quotas: vec![QuotaMsg { name: "daily".to_string(), duration: 1600, @@ -176,8 +183,9 @@ mod tests { execute(deps.as_mut(), env.clone(), info, msg).unwrap(); // remove the first one - let msg = ExecuteMsg::RemoveChannel { - channel_id: "channel".to_string(), + let msg = ExecuteMsg::RemovePath { + channel_id: format!("channel"), + denom: format!("denom"), }; let info = mock_info(IBC_ADDR, &vec![]); @@ -190,7 +198,8 @@ mod tests { // The second channel is still there let query_msg = QueryMsg::GetQuotas { - channel_id: "channel2".to_string(), + channel_id: format!("channel2"), + denom: format!("denom"), }; let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); let value: Vec = from_binary(&res).unwrap(); @@ -205,9 +214,10 @@ mod tests { env.block.time.plus_seconds(1600), ); - // Channels are overriden if they share a name - let msg = ExecuteMsg::AddChannel { - channel_id: "channel2".to_string(), + // Paths are overriden if they share a name and denom + let msg = ExecuteMsg::AddPath { + channel_id: format!("channel2"), + denom: format!("denom"), quotas: vec![QuotaMsg { name: "different".to_string(), duration: 5000, @@ -220,7 +230,8 @@ mod tests { execute(deps.as_mut(), env.clone(), info, msg).unwrap(); let query_msg = QueryMsg::GetQuotas { - channel_id: "channel2".to_string(), + channel_id: format!("channel2"), + denom: format!("denom"), }; let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); let value: Vec = from_binary(&res).unwrap(); diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index dcf3be447ef..5459e44eb99 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -4,11 +4,26 @@ use serde::{Deserialize, Serialize}; // The channel struct contains a name representing a unique identifier within ibc-go, and a list of rate limit quotas #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] -pub struct Channel { - pub name: String, +pub struct PathMsg { + pub channel_id: String, + pub denom: String, pub quotas: Vec, } +impl PathMsg { + pub fn new( + channel: impl Into, + denom: impl Into, + quotas: Vec, + ) -> Self { + PathMsg { + channel_id: channel.into(), + denom: denom.into(), + quotas, + } + } +} + #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct QuotaMsg { pub name: String, @@ -32,7 +47,7 @@ impl QuotaMsg { pub struct InstantiateMsg { pub gov_module: Addr, pub ibc_module: Addr, - pub channels: Vec, + pub paths: Vec, } /// The caller (IBC module) is responsibble for correctly calculating the funds @@ -42,23 +57,29 @@ pub struct InstantiateMsg { pub enum ExecuteMsg { SendPacket { channel_id: String, + denom: String, channel_value: u128, funds: u128, }, RecvPacket { channel_id: String, + denom: String, channel_value: u128, funds: u128, }, - AddChannel { + // TODO: rename these to AddPath and RemovePath + AddPath { channel_id: String, + denom: String, quotas: Vec, }, - RemoveChannel { + RemovePath { channel_id: String, + denom: String, }, - ResetChannelQuota { + ResetPathQuota { channel_id: String, + denom: String, quota_id: String, }, } @@ -66,7 +87,7 @@ pub enum ExecuteMsg { #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum QueryMsg { - GetQuotas { channel_id: String }, + GetQuotas { channel_id: String, denom: String }, } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] 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 bd6dd20b53a..557d1faa6a5 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -11,22 +11,50 @@ pub const RESET_TIME_DAILY: u64 = 60 * 60 * 24; pub const RESET_TIME_WEEKLY: u64 = 60 * 60 * 24 * 7; pub const RESET_TIME_MONTHLY: u64 = 60 * 60 * 24 * 30; +/// This represents +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] +pub struct Path { + pub denom: String, + pub channel: String, +} + +impl Path { + pub fn new(channel: impl Into, denom: impl Into) -> Self { + Path { + channel: channel.into(), + denom: denom.into(), + } + } +} + +impl Into<(String, String)> for Path { + fn into(self) -> (String, String) { + (self.channel.to_string(), self.denom.to_string()) + } +} + #[derive(Debug, Clone)] pub enum FlowType { In, Out, } -/// A Flow represents the transfer of value through an IBC channel during a -/// time window. +/// A Flow represents the transfer of value for a denom through an IBC channel +/// during a time window. /// /// It tracks inflows (transfers into osmosis) and outflows (transfers out of /// osmosis). /// -/// The period_end represents the last point in time that for which this Flow is +/// The period_end represents the last point in time for which this Flow is /// tracking the value transfer. -/// TODO: Document that time windows are not rolling windows, but instead discrete repeating windows. -/// This is a design decision chosen for gas reasons. +/// +/// Periods are discrete repeating windows. A period only starts when a contract +/// call to update the Flow (SendPacket/RecvPackt) is made, and not right after +/// the period ends. This means that if no calls happen after a period expires, +/// the next period will begin at the time of the next call and be valid for the +/// specified duration for the quota. +/// +/// This is a design decision to avoid the period calculations and thus reduce gas consumption #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema, Copy)] pub struct Flow { // Q: Do we have edge case issues with inflow/outflow being u128, e.g. what if a token has super high precision. @@ -49,8 +77,8 @@ impl Flow { } } - /// The balance of a flow is how much absolute value has moved through the - /// channel before period_end. + /// The balance of a flow is how much absolute value for the denom has moved + /// through the channel before period_end. pub fn balance(&self) -> u128 { self.inflow.abs_diff(self.outflow) } @@ -79,8 +107,8 @@ impl Flow { } } -/// A Quota is the percentage of the channel's total value that can be -/// transfered through the channel in a given period of time (duration) +/// A Quota is the percentage of the denom's total value that can be transfered +/// through the channel in a given period of time (duration) /// /// Percentages can be different for send and recv /// @@ -96,8 +124,8 @@ pub struct Quota { impl Quota { /// Calculates the max capacity (absolute value in the same unit as - /// total_value) in a given direction based on the total value of the - /// channel. + /// total_value) in a given direction based on the total value of the denom + /// in the channel. pub fn capacity_at(&self, total_value: &u128, direction: &FlowType) -> u128 { let max_percentage = match direction { FlowType::In => self.max_percentage_recv, @@ -122,7 +150,7 @@ impl From<&QuotaMsg> for Quota { } } -/// RateLimit is the main structure tracked for each channel. Its quota +/// RateLimit is the main structure tracked for each channel/denom pair. Its quota /// represents rate limit configuration, and the flow its /// current state (i.e.: how much value has been transfered in the current period) #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] @@ -138,19 +166,25 @@ pub const GOVMODULE: Item = Item::new("gov_module"); /// IBC transfer module, but could be set to something else if needed pub const IBCMODULE: Item = Item::new("ibc_module"); -/// RATE_LIMIT_TRACKERS is the main state for this contract. It maps an IBC channel_id -/// to a vector of `RateLimit`s. The `RateLimit` struct contains the information -/// about how much value has moved through the channel during the currently -/// active time period (channel_flow.flow) and what percentage of the channel's -/// value we are allowing to flow through that channel in a specific duration (quota) +/// RATE_LIMIT_TRACKERS is the main state for this contract. It maps a path (IBC +/// Channel + denom) to a vector of `RateLimit`s. +/// +/// The `RateLimit` struct contains the information about how much value of a +/// denom has moved through the channel during the currently active time period +/// (channel_flow.flow) and what percentage of the denom's value we are +/// allowing to flow through that channel in a specific duration (quota) /// -/// For simplicity, the map keys (ibc channel) refers to the "host" channel on the -/// osmosis side. This means that on PacketSend it will refer to the source +/// For simplicity, the channel in the map keys refers to the "host" channel on +/// the osmosis side. This means that on PacketSend it will refer to the source /// channel while on PacketRecv it refers to the destination channel. /// /// It is the responsibility of the go module to pass the appropriate channel /// when sending the messages -pub const RATE_LIMIT_TRACKERS: Map<&str, Vec> = Map::new("flow"); +/// +/// The map key (String, String) represents (channel_id, denom). We use +/// composite keys instead of a struct to avoid having to implement the +/// PrimaryKey trait +pub const RATE_LIMIT_TRACKERS: Map<(String, String), Vec> = Map::new("flow"); #[cfg(test)] mod tests { From 746daa930b0b62758227d502a375c92e540c8c83 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 17 Aug 2022 20:41:07 +0200 Subject: [PATCH 13/29] cleanup --- .../contracts/rate-limiter/src/contract.rs | 62 ++++++++++--------- .../contracts/rate-limiter/src/msg.rs | 4 +- .../contracts/rate-limiter/src/state.rs | 17 +++-- 3 files changed, 47 insertions(+), 36 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index b85d9028d1f..bc7ead614b0 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -46,31 +46,35 @@ pub fn execute( channel_value, funds, denom, - } => try_transfer( - deps, - info.sender, - channel_id, - denom, - channel_value, - funds, - FlowType::Out, - env.block.time, - ), + } => { + let path = Path::new(&channel_id, &denom); + try_transfer( + deps, + info.sender, + &path, + channel_value, + funds, + FlowType::Out, + env.block.time, + ) + } ExecuteMsg::RecvPacket { channel_id, channel_value, funds, denom, - } => try_transfer( - deps, - info.sender, - channel_id, - denom, - channel_value, - funds, - FlowType::In, - env.block.time, - ), + } => { + let path = Path::new(&channel_id, &denom); + try_transfer( + deps, + info.sender, + &path, + channel_value, + funds, + FlowType::In, + env.block.time, + ) + } ExecuteMsg::AddPath { channel_id, denom, @@ -112,8 +116,7 @@ pub struct RateLimitResponse { pub fn try_transfer( deps: DepsMut, sender: Addr, - channel_id: String, - denom: String, + path: &Path, channel_value: u128, funds: u128, direction: FlowType, @@ -128,7 +131,6 @@ pub fn try_transfer( return Err(ContractError::Unauthorized {}); } - let path = Path::new(&channel_id, &denom); let trackers = RATE_LIMIT_TRACKERS.may_load(deps.storage, path.into())?; let configured = match trackers { @@ -142,8 +144,8 @@ pub fn try_transfer( // TODO: Should there be an attribute for it being allowed vs denied? return Ok(Response::new() .add_attribute("method", "try_transfer") - .add_attribute("channel_id", channel_id) - .add_attribute("denom", denom) + .add_attribute("channel_id", path.channel.to_string()) + .add_attribute("denom", path.denom.to_string()) .add_attribute("quota", "none")); } @@ -167,8 +169,8 @@ pub fn try_transfer( let balance = limit.flow.balance(); if balance > max { return Err(ContractError::RateLimitExceded { - channel: channel_id.to_string(), - denom: denom.to_string(), + channel: path.channel.to_string(), + denom: path.denom.to_string(), reset: limit.flow.period_end, }); }; @@ -187,14 +189,14 @@ pub fn try_transfer( RATE_LIMIT_TRACKERS.save( deps.storage, - Path::new(&channel_id, &denom).into(), + path.into(), &results.iter().map(|r| r.rate_limit.clone()).collect(), )?; let response = Response::new() .add_attribute("method", "try_transfer") - .add_attribute("channel_id", channel_id) - .add_attribute("denom", denom); + .add_attribute("channel_id", path.channel.to_string()) + .add_attribute("denom", path.denom.to_string()); // Adding the attributes from each quota to the response // Code style Q: Should we move attribute setting to a function on response? diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index 5459e44eb99..b3fce3a6ff4 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -2,7 +2,7 @@ use cosmwasm_std::Addr; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -// The channel struct contains a name representing a unique identifier within ibc-go, and a list of rate limit quotas +// PathMsg contains a channel_id and denom to represent a unique identifier within ibc-go, and a list of rate limit quotas #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct PathMsg { pub channel_id: String, @@ -24,6 +24,7 @@ impl PathMsg { } } +// QuotaMsg represents a rate limiting Quota when sent as a wasm msg #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct QuotaMsg { pub name: String, @@ -67,7 +68,6 @@ pub enum ExecuteMsg { channel_value: u128, funds: u128, }, - // TODO: rename these to AddPath and RemovePath AddPath { channel_id: String, denom: String, 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 557d1faa6a5..d17b52884be 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -11,7 +11,10 @@ pub const RESET_TIME_DAILY: u64 = 60 * 60 * 24; pub const RESET_TIME_WEEKLY: u64 = 60 * 60 * 24 * 7; pub const RESET_TIME_MONTHLY: u64 = 60 * 60 * 24 * 30; -/// This represents +/// This represents the key for our rate limiting tracker. A tuple of a denom and +/// a channel. When interactic with storage, it's preffered to use this struct +/// and call path.into() on it to convert it to the composite key of the +/// RATE_LIMIT_TRACKERS map #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct Path { pub denom: String, @@ -27,9 +30,15 @@ impl Path { } } -impl Into<(String, String)> for Path { - fn into(self) -> (String, String) { - (self.channel.to_string(), self.denom.to_string()) +impl From for (String, String) { + fn from(path: Path) -> (String, String) { + (path.channel, path.denom) + } +} + +impl From<&Path> for (String, String) { + fn from(path: &Path) -> (String, String) { + (path.channel.to_owned(), path.denom.to_owned()) } } From fe97ee805477b096a9f7334bcbd0d7549521c6f1 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 10:02:44 +0200 Subject: [PATCH 14/29] refactoring --- .../contracts/rate-limiter/src/contract.rs | 47 ++------------- .../contracts/rate-limiter/src/state.rs | 60 ++++++++++++++++++- 2 files changed, 64 insertions(+), 43 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index bc7ead614b0..3c77b3770a0 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -8,7 +8,7 @@ use cw2::set_contract_version; use crate::error::ContractError; use crate::management::{add_new_paths, try_add_path, try_remove_path, try_reset_path_quota}; use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{FlowType, Path, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::state::{FlowType, Path, RateLimitResponse, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:rate-limiter"; @@ -98,12 +98,6 @@ pub fn execute( } } -pub struct RateLimitResponse { - pub rate_limit: RateLimit, - pub used: u128, - pub max: u128, -} - // Q: Is an ICS 20 transfer only 1 denom at a time, or does the caller have to split into several // calls if its a multi-denom ICS-20 transfer @@ -151,41 +145,10 @@ pub fn try_transfer( let mut rate_limits = trackers.unwrap(); - let results: Result, _> = rate_limits + let results: Vec = rate_limits .iter_mut() - .map(|limit| { - // TODO: Should we move this to more methods on ChannelFlow? - // e.g. new pseudocode - // channel.updateIfExpired(now) - // channel.trackSend(&direction, funds) - // channel.CheckRateLimit(direction.clone())?; - // (Or at least update for time + rename for track send. the last one is a bit of a code style nit) - let max = limit.quota.capacity_at(&channel_value, &direction); - if limit.flow.is_expired(now) { - limit.flow.expire(now, limit.quota.duration) - } - limit.flow.add_flow(direction.clone(), funds); - - let balance = limit.flow.balance(); - if balance > max { - return Err(ContractError::RateLimitExceded { - channel: path.channel.to_string(), - denom: path.denom.to_string(), - reset: limit.flow.period_end, - }); - }; - Ok(RateLimitResponse { - // TODO: nit, can we derive channel.Clone()? - rate_limit: RateLimit { - quota: limit.quota.clone(), - flow: limit.flow, - }, - used: balance, - max, - }) - }) - .collect(); - let results = results?; + .map(|limit| limit.allow_transfer(&path, &direction, funds, channel_value, now)) + .collect::>()?; RATE_LIMIT_TRACKERS.save( deps.storage, @@ -247,7 +210,7 @@ mod tests { use crate::helpers::tests::verify_query_response; use crate::msg::{PathMsg, QuotaMsg}; - use crate::state::RESET_TIME_WEEKLY; + use crate::state::{RateLimit, RESET_TIME_WEEKLY}; const IBC_ADDR: &str = "IBC_MODULE"; const GOV_ADDR: &str = "GOV_MODULE"; 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 d17b52884be..d84041fc53f 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -5,7 +5,7 @@ use std::cmp; use cw_storage_plus::{Item, Map}; -use crate::msg::QuotaMsg; +use crate::{msg::QuotaMsg, ContractError}; pub const RESET_TIME_DAILY: u64 = 60 * 60 * 24; pub const RESET_TIME_WEEKLY: u64 = 60 * 60 * 24 * 7; @@ -92,6 +92,11 @@ impl Flow { self.inflow.abs_diff(self.outflow) } + /// checks if the flow, in the current state, has exceeded a max allowance + pub fn exceeds(&self, max: u128) -> bool { + self.balance() > max + } + /// 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 @@ -114,6 +119,15 @@ impl Flow { FlowType::Out => self.outflow = self.outflow.saturating_add(value), } } + + /// Applies a transfer. If the Flow is expired (now > period_end), it will + /// reset it before applying the transfer. + fn apply_transfer(&mut self, direction: &FlowType, funds: u128, now: Timestamp, duration: u64) { + if self.is_expired(now) { + self.expire(now, duration) + } + self.add_flow(direction.clone(), funds); + } } /// A Quota is the percentage of the denom's total value that can be transfered @@ -168,6 +182,50 @@ pub struct RateLimit { pub flow: Flow, } +impl RateLimit { + /// Checks if a transfer is allowed and updates the data structures + /// accordingly. + /// + /// If the transfer is not allowed, it will return a RateLimitExceeded error. + /// + /// Otherwise it will return a RateLimitResponse with the updated data structures + pub fn allow_transfer( + &mut self, + path: &Path, + direction: &FlowType, + funds: u128, + channel_value: u128, + now: Timestamp, + ) -> Result { + self.flow + .apply_transfer(direction, funds, now, self.quota.duration); + let max = self.quota.capacity_at(&channel_value, &direction); + + // Return the effects of applying the transfer or an error. + match self.flow.exceeds(max) { + true => Err(ContractError::RateLimitExceded { + channel: path.channel.to_string(), + denom: path.denom.to_string(), + reset: self.flow.period_end, + }), + false => Ok(RateLimitResponse { + rate_limit: RateLimit { + quota: self.quota.clone(), // Cloning here because self.quota.name (String) does not allow us to implement Copy + flow: self.flow, // We can Copy flow, so this is slightly more efficient than cloning the whole RateLimit + }, + used: self.flow.balance(), + max, + }), + } + } +} + +pub struct RateLimitResponse { + pub rate_limit: RateLimit, + pub used: u128, + pub max: u128, +} + /// Only this address can manage the contract. This will likely be the /// governance module, but could be set to something else if needed pub const GOVMODULE: Item = Item::new("gov_module"); From c6758f199de6f3b6f595e24ff89ebbf1174a78fb Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 11:39:29 +0200 Subject: [PATCH 15/29] changes to the expiration logic so that balances are calculated based on the direction (as suggested by @ValarDragon) --- .../contracts/rate-limiter/src/contract.rs | 89 +++- .../rate-limiter/src/integration_tests.rs | 401 +++++++++--------- .../contracts/rate-limiter/src/state.rs | 51 ++- 3 files changed, 303 insertions(+), 238 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 3c77b3770a0..e3fd716a689 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -147,7 +147,7 @@ pub fn try_transfer( let results: Vec = rate_limits .iter_mut() - .map(|limit| limit.allow_transfer(&path, &direction, funds, channel_value, now)) + .map(|limit| limit.allow_transfer(path, &direction, funds, channel_value, now)) .collect::>()?; RATE_LIMIT_TRACKERS.save( @@ -164,15 +164,24 @@ pub fn try_transfer( // Adding the attributes from each quota to the response // Code style Q: Should we move attribute setting to a function on response? // Rust question: How does this work with one response being an error, I'm not sure how the flow works here + // TODO: Do we even need these attributes? The response is only ever seen by the chain. results.iter().fold(Ok(response), |acc, result| { Ok(acc? .add_attribute( - format!("{}_used", result.rate_limit.quota.name), - result.used.to_string(), + format!("{}_used_in", result.rate_limit.quota.name), + result.rate_limit.flow.balance().0.to_string(), ) .add_attribute( - format!("{}_max", result.rate_limit.quota.name), - result.max.to_string(), + format!("{}_used_out", result.rate_limit.quota.name), + result.rate_limit.flow.balance().1.to_string(), + ) + .add_attribute( + format!("{}_max_in", result.rate_limit.quota.name), + result.max_in.to_string(), + ) + .add_attribute( + format!("{}_max_out", result.rate_limit.quota.name), + result.max_out.to_string(), ) .add_attribute( format!("{}_period_end", result.rate_limit.quota.name), @@ -300,8 +309,8 @@ mod tests { let info = mock_info(IBC_ADDR, &vec![]); let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); assert_eq!(value, "300"); let msg = ExecuteMsg::SendPacket { @@ -346,13 +355,20 @@ mod tests { }; let res = execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); + println!("{:?}", res); let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used"); + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); assert_eq!(value, "300"); let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used"); + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); assert_eq!(value, "0"); // We can still use the path. Even if we have sent more than the @@ -360,8 +376,11 @@ mod tests { // of inflow vs outflow is still lower than the path's capacity/quota let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used"); + assert_eq!(key, "weekly_used_in"); assert_eq!(value, "300"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "0"); let err = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap_err(); @@ -372,7 +391,7 @@ mod tests { fn asymetric_quotas() { let mut deps = mock_dependencies(); - let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 1); + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 4, 1); let msg = InstantiateMsg { gov_module: Addr::unchecked(GOV_ADDR), ibc_module: Addr::unchecked(IBC_ADDR), @@ -394,34 +413,64 @@ mod tests { }; let info = mock_info(IBC_ADDR, &vec![]); let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); assert_eq!(value, "60"); - // Sending 1% more. Allowed, as sending has a 10% allowance + // Sending 2% more. Allowed, as sending has a 4% allowance let msg = ExecuteMsg::SendPacket { channel_id: format!("channel"), denom: format!("denom"), channel_value: 3_000, - funds: 30, + funds: 60, }; let info = mock_info(IBC_ADDR, &vec![]); let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used"); - assert_eq!(value, "90"); + println!("{res:?}"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "120"); - // Receiving 1% should fail. 3% already executed through the path + // Receiving 1% should still work. 4% *sent* through the path, but we can still receive. let recv_msg = ExecuteMsg::RecvPacket { channel_id: format!("channel"), denom: format!("denom"), channel_value: 3_000, funds: 30, }; + let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg).unwrap(); + let Attribute { key, value } = &res.attributes[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "90"); - let err = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap_err(); + // Sending 2%. Should fail. In balance, we've sent 4% and received 1%, so only 1% left to send. + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 60, + }; + let err = execute(deps.as_mut(), mock_env(), info.clone(), msg.clone()).unwrap_err(); assert!(matches!(err, ContractError::RateLimitExceded { .. })); + + // Sending 1%: Allowed; because sending has a 4% allowance. We've sent 4% already, but received 1%, so there's send cappacity again + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 30, + }; + let res = execute(deps.as_mut(), mock_env(), info.clone(), msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "120"); } #[test] 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 ba6c189ee86..896f27ff442 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 @@ -1,10 +1,14 @@ #[cfg(test)] mod tests { - use crate::msg::InstantiateMsg; - use crate::{helpers::RateLimitingContract, msg::PathMsg}; + use crate::helpers::RateLimitingContract; use cosmwasm_std::{Addr, Coin, Empty, Uint128}; use cw_multi_test::{App, AppBuilder, Contract, ContractWrapper, Executor}; + use crate::{ + msg::{ExecuteMsg, InstantiateMsg, PathMsg, QuotaMsg}, + state::{RESET_TIME_DAILY, RESET_TIME_MONTHLY, RESET_TIME_WEEKLY}, + }; + pub fn contract_template() -> Box> { let contract = ContractWrapper::new( crate::contract::execute, @@ -35,6 +39,7 @@ mod tests { }) } + // Instantiate the contract fn proper_instantiate(paths: Vec) -> (App, RateLimitingContract) { let mut app = mock_app(); let cw_template_id = app.store_code(contract_template()); @@ -61,117 +66,141 @@ mod tests { (app, cw_template_contract) } - mod expiration { - use cosmwasm_std::Attribute; - - use super::*; - use crate::{ - msg::{ExecuteMsg, PathMsg, QuotaMsg}, - state::{RESET_TIME_DAILY, RESET_TIME_MONTHLY, RESET_TIME_WEEKLY}, - }; - - #[test] - fn expiration() { - let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); - - let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![quota], - }]); - - // Using all the allowance - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - let Attribute { key, value } = &res.custom_attrs(1)[3]; - assert_eq!(key, "weekly_used"); - assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[4]; - assert_eq!(key, "weekly_max"); - assert_eq!(value, "300"); + use cosmwasm_std::Attribute; - // Another packet is rate limited - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // TODO: how do we check the error type here? - - // ... Time passes - app.update_block(|b| { - b.height += 1000; - b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) - }); - - // Sending the packet should work now - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; + #[test] // Checks that the RateLimit flows are expired properly when time passes + fn expiration() { + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - let Attribute { key, value } = &res.custom_attrs(1)[3]; - assert_eq!(key, "weekly_used"); - assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[4]; - assert_eq!(key, "weekly_max"); - assert_eq!(value, "300"); - } + let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![quota], + }]); - #[test] - fn multiple_quotas() { - let quotas = vec![ - QuotaMsg::new("daily", RESET_TIME_DAILY, 1, 1), - QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), - QuotaMsg::new("monthly", RESET_TIME_MONTHLY, 5, 5), - ]; + // Using all the allowance + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + let Attribute { key, value } = &res.custom_attrs(1)[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.custom_attrs(1)[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[5]; + assert_eq!(key, "weekly_max_in"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[6]; + assert_eq!(key, "weekly_max_out"); + assert_eq!(value, "300"); + + // Another packet is rate limited + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // TODO: how do we check the error type here? + + // ... Time passes + app.update_block(|b| { + b.height += 1000; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // Sending the packet should work now + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; - let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas, - }]); + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + let Attribute { key, value } = &res.custom_attrs(1)[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.custom_attrs(1)[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[5]; + assert_eq!(key, "weekly_max_in"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[6]; + assert_eq!(key, "weekly_max_out"); + assert_eq!(value, "300"); + } - // Sending 1% to use the daily allowance - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + #[test] + fn multiple_quotas() { + let quotas = vec![ + QuotaMsg::new("daily", RESET_TIME_DAILY, 1, 1), + QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), + QuotaMsg::new("monthly", RESET_TIME_MONTHLY, 5, 5), + ]; + + let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas, + }]); + + // Sending 1% to use the daily allowance + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + // Another packet is rate limited + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // Sending the packet should work now + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; - // Another packet is rate limited - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + // Do that for 4 more days + for _ in 1..4 { // ... One day passes app.update_block(|b| { b.height += 10; @@ -185,98 +214,78 @@ mod tests { channel_value: 100, funds: 1, }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - // Do that for 4 more days - for _ in 1..4 { - // ... One day passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) - }); - - // Sending the packet should work now - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - } - - // ... One day passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) - }); - - // We now have exceeded the weekly limit! Even if the daily limit allows us, the weekly doesn't - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // ... One week passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) - }); - - // We can still can't send because the weekly and monthly limits are the same - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // Waiting a week again, doesn't help!! - // ... One week passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) - }); - - // We can still can't send because the monthly limit hasn't passed - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // Only after two more weeks we can send again - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds((RESET_TIME_WEEKLY * 2) + 1) // Two weeks - }); - - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); } + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // We now have exceeded the weekly limit! Even if the daily limit allows us, the weekly doesn't + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // We can still can't send because the weekly and monthly limits are the same + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // Waiting a week again, doesn't help!! + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // We can still can't send because the monthly limit hasn't passed + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // Only after two more weeks we can send again + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds((RESET_TIME_WEEKLY * 2) + 1) // Two weeks + }); + + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); } } 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 d84041fc53f..e43f56dd07f 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -88,13 +88,20 @@ impl Flow { /// The balance of a flow is how much absolute value for the denom has moved /// through the channel before period_end. - pub fn balance(&self) -> u128 { - self.inflow.abs_diff(self.outflow) + pub fn balance(&self) -> (u128, u128) { + ( + self.inflow.saturating_sub(self.outflow), + self.outflow.saturating_sub(self.inflow), + ) } /// checks if the flow, in the current state, has exceeded a max allowance - pub fn exceeds(&self, max: u128) -> bool { - self.balance() > max + pub fn exceeds(&self, direction: &FlowType, max_inflow: u128, max_outflow: u128) -> bool { + let (balance_in, balance_out) = self.balance(); + match direction { + FlowType::In => balance_in > max_inflow, + FlowType::Out => balance_out > max_outflow, + } } /// If now is greater than the period_end, the Flow is considered expired. @@ -147,14 +154,14 @@ pub struct Quota { impl Quota { /// Calculates the max capacity (absolute value in the same unit as - /// total_value) in a given direction based on the total value of the denom - /// in the channel. - pub fn capacity_at(&self, total_value: &u128, direction: &FlowType) -> u128 { - let max_percentage = match direction { - FlowType::In => self.max_percentage_recv, - FlowType::Out => self.max_percentage_send, - }; - total_value * (max_percentage as u128) / 100_u128 + /// total_value) in each direction based on the total value of the denom in + /// the channel. The result tuple represents the max capacity when the + /// transfer is in directions: (FlowType::In, FlowType::Out) + pub fn capacity_at(&self, total_value: &u128) -> (u128, u128) { + ( + total_value * (self.max_percentage_recv as u128) / 100_u128, + total_value * (self.max_percentage_send as u128) / 100_u128, + ) } } @@ -199,10 +206,10 @@ impl RateLimit { ) -> Result { self.flow .apply_transfer(direction, funds, now, self.quota.duration); - let max = self.quota.capacity_at(&channel_value, &direction); + let (max_in, max_out) = self.quota.capacity_at(&channel_value); // Return the effects of applying the transfer or an error. - match self.flow.exceeds(max) { + match self.flow.exceeds(direction, max_in, max_out) { true => Err(ContractError::RateLimitExceded { channel: path.channel.to_string(), denom: path.denom.to_string(), @@ -213,8 +220,8 @@ impl RateLimit { quota: self.quota.clone(), // Cloning here because self.quota.name (String) does not allow us to implement Copy flow: self.flow, // We can Copy flow, so this is slightly more efficient than cloning the whole RateLimit }, - used: self.flow.balance(), - max, + max_in, + max_out, }), } } @@ -222,8 +229,8 @@ impl RateLimit { pub struct RateLimitResponse { pub rate_limit: RateLimit, - pub used: u128, - pub max: u128, + pub max_in: u128, + pub max_out: u128, } /// Only this address can manage the contract. This will likely be the @@ -267,16 +274,16 @@ mod tests { assert!(!flow.is_expired(epoch.plus_seconds(RESET_TIME_WEEKLY))); assert!(flow.is_expired(epoch.plus_seconds(RESET_TIME_WEEKLY).plus_nanos(1))); - assert_eq!(flow.balance(), 0_u128); + assert_eq!(flow.balance(), (0_u128, 0_u128)); flow.add_flow(FlowType::In, 5); - assert_eq!(flow.balance(), 5_u128); + assert_eq!(flow.balance(), (5_u128, 0_u128)); flow.add_flow(FlowType::Out, 2); - assert_eq!(flow.balance(), 3_u128); + assert_eq!(flow.balance(), (3_u128, 0_u128)); // Adding flow doesn't affect expiration assert!(!flow.is_expired(epoch.plus_seconds(RESET_TIME_DAILY))); flow.expire(epoch.plus_seconds(RESET_TIME_WEEKLY), RESET_TIME_WEEKLY); - assert_eq!(flow.balance(), 0_u128); + assert_eq!(flow.balance(), (0_u128, 0_u128)); assert_eq!(flow.inflow, 0_u128); assert_eq!(flow.outflow, 0_u128); assert_eq!(flow.period_end, epoch.plus_seconds(RESET_TIME_WEEKLY * 2)); From e1d52f40bac8c7ffad69c9f2a400f152834eb162 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 12:16:16 +0200 Subject: [PATCH 16/29] slightly slower but considerably cleanner --- .../contracts/rate-limiter/src/contract.rs | 39 +++++++++---------- .../contracts/rate-limiter/src/state.rs | 18 ++------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index e3fd716a689..9f9e19799eb 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -8,7 +8,7 @@ use cw2::set_contract_version; use crate::error::ContractError; use crate::management::{add_new_paths, try_add_path, try_remove_path, try_reset_path_quota}; use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{FlowType, Path, RateLimitResponse, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::state::{FlowType, Path, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:rate-limiter"; @@ -145,16 +145,14 @@ pub fn try_transfer( let mut rate_limits = trackers.unwrap(); - let results: Vec = rate_limits + // If any of the RateLimits fails, allow_transfer() will return + // ContractError::RateLimitExceded, which we'll propagate out + let results: Vec = rate_limits .iter_mut() .map(|limit| limit.allow_transfer(path, &direction, funds, channel_value, now)) .collect::>()?; - RATE_LIMIT_TRACKERS.save( - deps.storage, - path.into(), - &results.iter().map(|r| r.rate_limit.clone()).collect(), - )?; + RATE_LIMIT_TRACKERS.save(deps.storage, path.into(), &results)?; let response = Response::new() .add_attribute("method", "try_transfer") @@ -164,28 +162,29 @@ pub fn try_transfer( // Adding the attributes from each quota to the response // Code style Q: Should we move attribute setting to a function on response? // Rust question: How does this work with one response being an error, I'm not sure how the flow works here - // TODO: Do we even need these attributes? The response is only ever seen by the chain. results.iter().fold(Ok(response), |acc, result| { + // TODO: Do we even need these attributes? The response is only ever + // seen by the chain. Maybe we could add them conditional on a feature + // (debug, or testing) + let (used_in, used_out) = result.flow.balance(); + let (max_in, max_out) = result.quota.capacity_at(&channel_value); Ok(acc? .add_attribute( - format!("{}_used_in", result.rate_limit.quota.name), - result.rate_limit.flow.balance().0.to_string(), + format!("{}_used_in", result.quota.name), + used_in.to_string(), ) .add_attribute( - format!("{}_used_out", result.rate_limit.quota.name), - result.rate_limit.flow.balance().1.to_string(), + format!("{}_used_out", result.quota.name), + used_out.to_string(), ) + .add_attribute(format!("{}_max_in", result.quota.name), max_in.to_string()) .add_attribute( - format!("{}_max_in", result.rate_limit.quota.name), - result.max_in.to_string(), + format!("{}_max_out", result.quota.name), + max_out.to_string(), ) .add_attribute( - format!("{}_max_out", result.rate_limit.quota.name), - result.max_out.to_string(), - ) - .add_attribute( - format!("{}_period_end", result.rate_limit.quota.name), - result.rate_limit.flow.period_end.to_string(), + format!("{}_period_end", result.quota.name), + result.flow.period_end.to_string(), )) }) } 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 e43f56dd07f..f699c7d2e5a 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -203,7 +203,7 @@ impl RateLimit { funds: u128, channel_value: u128, now: Timestamp, - ) -> Result { + ) -> Result { self.flow .apply_transfer(direction, funds, now, self.quota.duration); @@ -215,24 +215,14 @@ impl RateLimit { denom: path.denom.to_string(), reset: self.flow.period_end, }), - false => Ok(RateLimitResponse { - rate_limit: RateLimit { - quota: self.quota.clone(), // Cloning here because self.quota.name (String) does not allow us to implement Copy - flow: self.flow, // We can Copy flow, so this is slightly more efficient than cloning the whole RateLimit - }, - max_in, - max_out, + false => Ok(RateLimit { + quota: self.quota.clone(), // Cloning here because self.quota.name (String) does not allow us to implement Copy + flow: self.flow, // We can Copy flow, so this is slightly more efficient than cloning the whole RateLimit }), } } } -pub struct RateLimitResponse { - pub rate_limit: RateLimit, - pub max_in: u128, - pub max_out: u128, -} - /// Only this address can manage the contract. This will likely be the /// governance module, but could be set to something else if needed pub const GOVMODULE: Item = Item::new("gov_module"); From 7434a4477fc59714b8a8f261ac6fabc7cb545c79 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 12:34:25 +0200 Subject: [PATCH 17/29] cleanup attributes and removed redundancy when not testing --- .../contracts/rate-limiter/src/contract.rs | 69 ++++++++++++------- 1 file changed, 43 insertions(+), 26 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 9f9e19799eb..a32f9802988 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -159,36 +159,53 @@ pub fn try_transfer( .add_attribute("channel_id", path.channel.to_string()) .add_attribute("denom", path.denom.to_string()); - // Adding the attributes from each quota to the response - // Code style Q: Should we move attribute setting to a function on response? - // Rust question: How does this work with one response being an error, I'm not sure how the flow works here + // Adds the attributes for each path to the response. In prod, the + // addtribute add_rate_limit_attributes is a noop results.iter().fold(Ok(response), |acc, result| { - // TODO: Do we even need these attributes? The response is only ever - // seen by the chain. Maybe we could add them conditional on a feature - // (debug, or testing) - let (used_in, used_out) = result.flow.balance(); - let (max_in, max_out) = result.quota.capacity_at(&channel_value); - Ok(acc? - .add_attribute( - format!("{}_used_in", result.quota.name), - used_in.to_string(), - ) - .add_attribute( - format!("{}_used_out", result.quota.name), - used_out.to_string(), - ) - .add_attribute(format!("{}_max_in", result.quota.name), max_in.to_string()) - .add_attribute( - format!("{}_max_out", result.quota.name), - max_out.to_string(), - ) - .add_attribute( - format!("{}_period_end", result.quota.name), - result.flow.period_end.to_string(), - )) + Ok(add_rate_limit_attributes(acc?, result, channel_value)) }) } +#[cfg(test)] +pub fn add_rate_limit_attributes( + response: Response, + result: &RateLimit, + channel_value: u128, +) -> Response { + let (used_in, used_out) = result.flow.balance(); + let (max_in, max_out) = result.quota.capacity_at(&channel_value); + // These attributes are only added during testing. That way we avoid + // calculating these again on prod. + // TODO: Figure out how to include these when testing on the go side. + response + .add_attribute( + format!("{}_used_in", result.quota.name), + used_in.to_string(), + ) + .add_attribute( + format!("{}_used_out", result.quota.name), + used_out.to_string(), + ) + .add_attribute(format!("{}_max_in", result.quota.name), max_in.to_string()) + .add_attribute( + format!("{}_max_out", result.quota.name), + max_out.to_string(), + ) + .add_attribute( + format!("{}_period_end", result.quota.name), + result.flow.period_end.to_string(), + ) +} + +#[cfg(not(test))] +pub fn add_rate_limit_attributes( + response: Response, + _result: &RateLimit, + _channel_value: u128, +) -> Response { + response +} + #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { From 727fef20877f52d0ff936ec16ac0cd6e4aab578b Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 13:17:08 +0200 Subject: [PATCH 18/29] update to edition 2021 --- x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml b/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml index a94d596a72c..b1b53b54bf1 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml +++ b/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml @@ -2,7 +2,7 @@ name = "rate-limiter" version = "0.1.0" authors = ["Nicolas Lara "] -edition = "2018" +edition = "2021" exclude = [ # Those files are rust-optimizer artifacts. You might want to commit them for convenience but they should not be part of the source code publication. From 7859a554a6ee122520f75ffec72297a3a1e69c42 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 14:00:49 +0200 Subject: [PATCH 19/29] added comments explaining the tests --- .../contracts/rate-limiter/src/contract.rs | 16 +++++++--------- .../rate-limiter/src/integration_tests.rs | 2 +- .../contracts/rate-limiter/src/management.rs | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index a32f9802988..80589fd5581 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -240,7 +240,7 @@ mod tests { const IBC_ADDR: &str = "IBC_MODULE"; const GOV_ADDR: &str = "GOV_MODULE"; - #[test] + #[test] // Tests we ccan instantiate the contract and that the owners are set correctly fn proper_instantiation() { let mut deps = mock_dependencies(); @@ -260,7 +260,7 @@ mod tests { assert_eq!(GOVMODULE.load(deps.as_ref().storage).unwrap(), GOV_ADDR); } - #[test] + #[test] // Tests only the IBC_MODULE address can execute send and recv packet fn permissions() { let mut deps = mock_dependencies(); @@ -299,7 +299,7 @@ mod tests { assert!(matches!(err, ContractError::Unauthorized { .. })); } - #[test] + #[test] // Tests that when a packet is transferred, the peropper allowance is consummed fn consume_allowance() { let mut deps = mock_dependencies(); @@ -339,7 +339,7 @@ mod tests { assert!(matches!(err, ContractError::RateLimitExceded { .. })); } - #[test] + #[test] // Tests that the balance of send and receive is maintained (i.e: recives are sustracted from the send allowance and sends from the receives) fn symetric_flows_dont_consume_allowance() { let mut deps = mock_dependencies(); @@ -371,7 +371,6 @@ mod tests { }; let res = execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); - println!("{:?}", res); let Attribute { key, value } = &res.attributes[3]; assert_eq!(key, "weekly_used_in"); assert_eq!(value, "0"); @@ -403,7 +402,7 @@ mod tests { assert!(matches!(err, ContractError::RateLimitExceded { .. })); } - #[test] + #[test] // Tests that we can have different quotas for send and receive. In this test we use 4% send and 1% receive fn asymetric_quotas() { let mut deps = mock_dependencies(); @@ -489,7 +488,7 @@ mod tests { assert_eq!(value, "120"); } - #[test] + #[test] // Tests we can get the current state of the trackers fn query_state() { let mut deps = mock_dependencies(); @@ -556,7 +555,7 @@ mod tests { ); } - #[test] + #[test] // Tests quota percentages are between [0,100] fn bad_quotas() { let mut deps = mock_dependencies(); @@ -575,7 +574,6 @@ mod tests { }; let info = mock_info(IBC_ADDR, &vec![]); - // we can just call .unwrap() to assert this was a success let env = mock_env(); instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); 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 896f27ff442..c453caa4023 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 @@ -146,7 +146,7 @@ mod tests { assert_eq!(value, "300"); } - #[test] + #[test] // Tests we can have different maximums for different quotaas (daily, weekly, etc) and that they all are active at the same time fn multiple_quotas() { let quotas = vec![ QuotaMsg::new("daily", RESET_TIME_DAILY, 1, 1), diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs index e54377f02af..04dc47df802 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/management.rs @@ -122,8 +122,8 @@ mod tests { const IBC_ADDR: &str = "IBC_MODULE"; const GOV_ADDR: &str = "GOV_MODULE"; - #[test] - fn management_add_and_remove_channel() { + #[test] // Tests AddPath and RemovePath messages + fn management_add_and_remove_path() { let mut deps = mock_dependencies(); IBCMODULE .save(deps.as_mut().storage, &Addr::unchecked(IBC_ADDR)) From 628db471a194f4e6d7569fe9f597e5f1bc0df8ed Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 14:04:26 +0200 Subject: [PATCH 20/29] removed .beaker --- x/ibc-rate-limit/.beaker/state.json | 1 - x/ibc-rate-limit/.gitignore | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 x/ibc-rate-limit/.beaker/state.json diff --git a/x/ibc-rate-limit/.beaker/state.json b/x/ibc-rate-limit/.beaker/state.json deleted file mode 100644 index 9e26dfeeb6e..00000000000 --- a/x/ibc-rate-limit/.beaker/state.json +++ /dev/null @@ -1 +0,0 @@ -{} \ No newline at end of file diff --git a/x/ibc-rate-limit/.gitignore b/x/ibc-rate-limit/.gitignore index 0814c1f8964..631a00aa67f 100644 --- a/x/ibc-rate-limit/.gitignore +++ b/x/ibc-rate-limit/.gitignore @@ -17,5 +17,5 @@ Cargo.lock *.pdb -# Ignores local beaker state -**/state.local.json +# Ignores beaker state +**/.beaker From 9facb2797140e548579241452a94971372926c8c Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 14:06:20 +0200 Subject: [PATCH 21/29] unified gitignore --- .gitignore | 23 ++++++++++++++++++++++- x/ibc-rate-limit/.gitignore | 21 --------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/.gitignore b/.gitignore index 191166787bd..b291e13796e 100644 --- a/.gitignore +++ b/.gitignore @@ -205,4 +205,25 @@ tools-stamp *.save *.save.* -mutation_test_result.txt \ No newline at end of file +mutation_test_result.txt + +# Rust ignores. Generated by Cargo +# will have compiled files and executables +debug/ +target/ + +# Generated by rust-optimizer +artifacts/ + +# Remove Cargo.lock from gitignore if creating an executable, leave it for libraries +# More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html +Cargo.lock + +# These are backup files generated by rustfmt +**/*.rs.bk + +# MSVC Windows builds of rustc generate these, which store debugging information +*.pdb + +# Ignores beaker state +.beaker diff --git a/x/ibc-rate-limit/.gitignore b/x/ibc-rate-limit/.gitignore index 631a00aa67f..e69de29bb2d 100644 --- a/x/ibc-rate-limit/.gitignore +++ b/x/ibc-rate-limit/.gitignore @@ -1,21 +0,0 @@ -# Generated by Cargo -# will have compiled files and executables -debug/ -target/ - -# Generated by rust-optimizer -artifacts/ - -# Remove Cargo.lock from gitignore if creating an executable, leave it for libraries -# More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html -Cargo.lock - -# These are backup files generated by rustfmt -**/*.rs.bk - -# MSVC Windows builds of rustc generate these, which store debugging information -*.pdb - - -# Ignores beaker state -**/.beaker From 042f3f4eeb8b40a12f5c0483fbc313a399dfa9a4 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 14:06:37 +0200 Subject: [PATCH 22/29] removed second gitignore --- x/ibc-rate-limit/.gitignore | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 x/ibc-rate-limit/.gitignore diff --git a/x/ibc-rate-limit/.gitignore b/x/ibc-rate-limit/.gitignore deleted file mode 100644 index e69de29bb2d..00000000000 From c37a9689e2406b264e91b13464ed3d6edb68e662 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 14:15:51 +0200 Subject: [PATCH 23/29] better doc comments --- x/ibc-rate-limit/contracts/rate-limiter/src/state.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 f699c7d2e5a..22ef5005d16 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -87,7 +87,10 @@ impl Flow { } /// The balance of a flow is how much absolute value for the denom has moved - /// through the channel before period_end. + /// through the channel before period_end. It returns a tuple of + /// (balance_in, balance_out) where balance_in in is how much has been + /// transferred into the flow, and balance_out is how much value transferred + /// out. pub fn balance(&self) -> (u128, u128) { ( self.inflow.saturating_sub(self.outflow), From e5d72d65b93c8b5f2ee6b61e0c16b5ef3424ebda Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 14:17:17 +0200 Subject: [PATCH 24/29] spelling --- x/ibc-rate-limit/contracts/rate-limiter/src/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 22ef5005d16..97cf961abd5 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -140,7 +140,7 @@ impl Flow { } } -/// A Quota is the percentage of the denom's total value that can be transfered +/// A Quota is the percentage of the denom's total value that can be transferred /// through the channel in a given period of time (duration) /// /// Percentages can be different for send and recv From d658fddb759abeac10b08fecd3c98f82d76a28da Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 14:18:46 +0200 Subject: [PATCH 25/29] spelling --- x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index b3fce3a6ff4..be0ba35e2f9 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -51,7 +51,7 @@ pub struct InstantiateMsg { pub paths: Vec, } -/// The caller (IBC module) is responsibble for correctly calculating the funds +/// The caller (IBC module) is responsible for correctly calculating the funds /// being sent through the channel #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] From c2a72add8d1accaf188b7a61b296c885170dc3cc Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Thu, 18 Aug 2022 18:28:58 +0200 Subject: [PATCH 26/29] added channel value cache --- .../contracts/rate-limiter/src/contract.rs | 16 +-- .../rate-limiter/src/integration_tests.rs | 98 +++++++++++++++++++ .../contracts/rate-limiter/src/state.rs | 40 ++++++-- 3 files changed, 132 insertions(+), 22 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 80589fd5581..40879e70c6f 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -162,18 +162,14 @@ pub fn try_transfer( // Adds the attributes for each path to the response. In prod, the // addtribute add_rate_limit_attributes is a noop results.iter().fold(Ok(response), |acc, result| { - Ok(add_rate_limit_attributes(acc?, result, channel_value)) + Ok(add_rate_limit_attributes(acc?, result)) }) } #[cfg(test)] -pub fn add_rate_limit_attributes( - response: Response, - result: &RateLimit, - channel_value: u128, -) -> Response { +pub fn add_rate_limit_attributes(response: Response, result: &RateLimit) -> Response { let (used_in, used_out) = result.flow.balance(); - let (max_in, max_out) = result.quota.capacity_at(&channel_value); + let (max_in, max_out) = result.quota.capacity(); // These attributes are only added during testing. That way we avoid // calculating these again on prod. // TODO: Figure out how to include these when testing on the go side. @@ -198,11 +194,7 @@ pub fn add_rate_limit_attributes( } #[cfg(not(test))] -pub fn add_rate_limit_attributes( - response: Response, - _result: &RateLimit, - _channel_value: u128, -) -> Response { +pub fn add_rate_limit_attributes(response: Response, _result: &RateLimit) -> Response { response } 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 c453caa4023..3c4ccdae697 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 @@ -288,4 +288,102 @@ mod tests { let cosmos_msg = cw_template_contract.call(msg).unwrap(); let _err = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); } + + #[test] // Tests that the channel value is based on the value at the beginning of the period + fn channel_value_cached() { + let quotas = vec![ + QuotaMsg::new("daily", RESET_TIME_DAILY, 2, 2), + QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), + ]; + + let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas, + }]); + + // Sending 1% (half of the daily allowance) + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + // Sending 3% is now rate limited + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 3, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // Even if the channel value increases, the percentage is calculated based on the value at period start + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100000, + funds: 3, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // New Channel Value world! + + // Sending 1% of a new value (10_000) passes the daily check, cause it + // has expired, but not the weekly check (The value for last week is + // sitll 100, as only 1 day has passed) + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 10_000, + funds: 100, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // Sending 1% of a new value should work and set the value for the day at 10_000 + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 10_000, + funds: 100, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + // If the value magically decreasses. We can still send up to 100 more (1% of 10k) + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 1, + funds: 75, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + } } 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 97cf961abd5..5ba5c4ced78 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -132,11 +132,20 @@ impl Flow { /// Applies a transfer. If the Flow is expired (now > period_end), it will /// reset it before applying the transfer. - fn apply_transfer(&mut self, direction: &FlowType, funds: u128, now: Timestamp, duration: u64) { + fn apply_transfer( + &mut self, + direction: &FlowType, + funds: u128, + now: Timestamp, + quota: &Quota, + ) -> bool { + let mut expired = false; if self.is_expired(now) { - self.expire(now, duration) + self.expire(now, quota.duration); + expired = true; } self.add_flow(direction.clone(), funds); + expired } } @@ -153,6 +162,7 @@ pub struct Quota { pub max_percentage_send: u32, pub max_percentage_recv: u32, pub duration: u64, + pub channel_value: Option, } impl Quota { @@ -160,11 +170,14 @@ impl Quota { /// total_value) in each direction based on the total value of the denom in /// the channel. The result tuple represents the max capacity when the /// transfer is in directions: (FlowType::In, FlowType::Out) - pub fn capacity_at(&self, total_value: &u128) -> (u128, u128) { - ( - total_value * (self.max_percentage_recv as u128) / 100_u128, - total_value * (self.max_percentage_send as u128) / 100_u128, - ) + pub fn capacity(&self) -> (u128, u128) { + match self.channel_value { + Some(total_value) => ( + total_value * (self.max_percentage_recv as u128) / 100_u128, + total_value * (self.max_percentage_send as u128) / 100_u128, + ), + None => (0, 0), // This should never happen, but ig the channel value is not set, we disallow any transfer + } } } @@ -179,6 +192,7 @@ impl From<&QuotaMsg> for Quota { max_percentage_send: send_recv.0, max_percentage_recv: send_recv.1, duration: msg.duration, + channel_value: None, } } } @@ -207,10 +221,16 @@ impl RateLimit { channel_value: u128, now: Timestamp, ) -> Result { - self.flow - .apply_transfer(direction, funds, now, self.quota.duration); + let expired = self.flow.apply_transfer(direction, funds, now, &self.quota); + println!("EXPIRED: {expired}"); + // Cache the channel value if it has never been set or it has expired. + if self.quota.channel_value.is_none() || expired { + println!("CHANNEL_VALUE OLD: {:?}", self.quota.channel_value); + println!("CHANNEL_VALUE NEW: {}", channel_value); + self.quota.channel_value = Some(channel_value) + } - let (max_in, max_out) = self.quota.capacity_at(&channel_value); + let (max_in, max_out) = self.quota.capacity(); // Return the effects of applying the transfer or an error. match self.flow.exceeds(direction, max_in, max_out) { true => Err(ContractError::RateLimitExceded { From 43d6f17a9d03c83328dff6c4ca8a98dafa5c7529 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Fri, 19 Aug 2022 13:31:13 +0200 Subject: [PATCH 27/29] leaving the attributes hard-coded until they can be properly configured in CI --- x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml | 3 +++ .../contracts/rate-limiter/src/contract.rs | 14 ++++++++------ .../contracts/rate-limiter/src/state.rs | 3 --- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml b/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml index b1b53b54bf1..e166d606418 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml +++ b/x/ibc-rate-limit/contracts/rate-limiter/Cargo.toml @@ -31,6 +31,9 @@ overflow-checks = true backtraces = ["cosmwasm-std/backtraces"] # use library feature to disable all instantiate/execute/query exports library = [] +# Use the verbose responses feature if you want to include information about +# the remaining quotas in the SendPacket/RecvPacket responses +verbose_responses = [] [package.metadata.scripts] optimize = """docker run --rm -v "$(pwd)":/code \ diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 40879e70c6f..0c61a4d185d 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -166,13 +166,12 @@ pub fn try_transfer( }) } -#[cfg(test)] +// #[cfg(any(feature = "verbose_responses", test))] pub fn add_rate_limit_attributes(response: Response, result: &RateLimit) -> Response { let (used_in, used_out) = result.flow.balance(); let (max_in, max_out) = result.quota.capacity(); // These attributes are only added during testing. That way we avoid // calculating these again on prod. - // TODO: Figure out how to include these when testing on the go side. response .add_attribute( format!("{}_used_in", result.quota.name), @@ -193,10 +192,13 @@ pub fn add_rate_limit_attributes(response: Response, result: &RateLimit) -> Resp ) } -#[cfg(not(test))] -pub fn add_rate_limit_attributes(response: Response, _result: &RateLimit) -> Response { - response -} +// Leaving the attributes in until we can conditionally compile the contract +// for the go tests in CI: https://github.com/mandrean/cw-optimizoor/issues/19 +// +// #[cfg(not(any(feature = "verbose_responses", test)))] +// pub fn add_rate_limit_attributes(response: Response, _result: &RateLimit) -> Response { +// response +// } #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { 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 5ba5c4ced78..b534ccfd1b1 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -222,11 +222,8 @@ impl RateLimit { now: Timestamp, ) -> Result { let expired = self.flow.apply_transfer(direction, funds, now, &self.quota); - println!("EXPIRED: {expired}"); // Cache the channel value if it has never been set or it has expired. if self.quota.channel_value.is_none() || expired { - println!("CHANNEL_VALUE OLD: {:?}", self.quota.channel_value); - println!("CHANNEL_VALUE NEW: {}", channel_value); self.quota.channel_value = Some(channel_value) } From acc413b38add94d1b47017db1a8f44152c33141f Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Mon, 22 Aug 2022 12:52:11 +0200 Subject: [PATCH 28/29] cleanup and reorganization --- .../contracts/rate-limiter/src/contract.rs | 550 +------------- .../rate-limiter/src/contract_tests.rs | 324 ++++++++ .../contracts/rate-limiter/src/execute.rs | 249 ++++++ .../contracts/rate-limiter/src/helpers.rs | 11 +- .../rate-limiter/src/integration_tests.rs | 714 +++++++++--------- .../contracts/rate-limiter/src/lib.rs | 16 +- .../contracts/rate-limiter/src/msg.rs | 29 +- .../contracts/rate-limiter/src/query.rs | 12 + .../contracts/rate-limiter/src/state.rs | 10 +- .../contracts/rate-limiter/src/sudo.rs | 95 +++ 10 files changed, 1125 insertions(+), 885 deletions(-) create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/contract_tests.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/execute.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/query.rs create mode 100644 x/ibc-rate-limit/contracts/rate-limiter/src/sudo.rs diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 0c61a4d185d..c42c88e8cb0 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -1,14 +1,12 @@ #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; -use cosmwasm_std::{ - to_binary, Addr, Binary, Deps, DepsMut, Env, MessageInfo, Response, StdResult, Timestamp, -}; +use cosmwasm_std::{Binary, Deps, DepsMut, Env, MessageInfo, Response, StdResult}; use cw2::set_contract_version; use crate::error::ContractError; -use crate::management::{add_new_paths, try_add_path, try_remove_path, try_reset_path_quota}; -use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{FlowType, Path, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg, SudoMsg}; +use crate::state::{FlowType, Path, GOVMODULE, IBCMODULE}; +use crate::{execute, query, sudo}; // version info for migration info const CONTRACT_NAME: &str = "crates.io:rate-limiter"; @@ -25,7 +23,7 @@ pub fn instantiate( IBCMODULE.save(deps.storage, &msg.ibc_module)?; GOVMODULE.save(deps.storage, &msg.gov_module)?; - add_new_paths(deps, msg.paths, env.block.time)?; + execute::add_new_paths(deps, msg.paths, env.block.time)?; Ok(Response::new() .add_attribute("method", "instantiate") @@ -41,16 +39,41 @@ pub fn execute( msg: ExecuteMsg, ) -> Result { match msg { - ExecuteMsg::SendPacket { + ExecuteMsg::AddPath { + channel_id, + denom, + quotas, + } => execute::try_add_path(deps, info.sender, channel_id, denom, quotas, env.block.time), + ExecuteMsg::RemovePath { channel_id, denom } => { + execute::try_remove_path(deps, info.sender, channel_id, denom) + } + ExecuteMsg::ResetPathQuota { + channel_id, + denom, + quota_id, + } => execute::try_reset_path_quota( + deps, + info.sender, + channel_id, + denom, + quota_id, + env.block.time, + ), + } +} + +#[entry_point] +pub fn sudo(deps: DepsMut, env: Env, msg: SudoMsg) -> Result { + match msg { + SudoMsg::SendPacket { channel_id, channel_value, funds, denom, } => { let path = Path::new(&channel_id, &denom); - try_transfer( + sudo::try_transfer( deps, - info.sender, &path, channel_value, funds, @@ -58,16 +81,15 @@ pub fn execute( env.block.time, ) } - ExecuteMsg::RecvPacket { + SudoMsg::RecvPacket { channel_id, channel_value, funds, denom, } => { let path = Path::new(&channel_id, &denom); - try_transfer( + sudo::try_transfer( deps, - info.sender, &path, channel_value, funds, @@ -75,517 +97,17 @@ pub fn execute( env.block.time, ) } - ExecuteMsg::AddPath { - channel_id, - denom, - quotas, - } => try_add_path(deps, info.sender, channel_id, denom, quotas, env.block.time), - ExecuteMsg::RemovePath { channel_id, denom } => { - try_remove_path(deps, info.sender, channel_id, denom) - } - ExecuteMsg::ResetPathQuota { - channel_id, - denom, - quota_id, - } => try_reset_path_quota( - deps, - info.sender, - channel_id, - denom, - quota_id, - env.block.time, - ), - } -} - -// Q: Is an ICS 20 transfer only 1 denom at a time, or does the caller have to split into several -// calls if its a multi-denom ICS-20 transfer - -/// This function checks the rate limit and, if successful, stores the updated data about the value -/// that has been transfered through the channel for a specific denom. -/// If the period for a RateLimit has ended, the Flow information is reset. -/// -/// The channel_value is the current value of the denom for the the channel as -/// calculated by the caller. This should be the total supply of a denom -pub fn try_transfer( - deps: DepsMut, - sender: Addr, - path: &Path, - channel_value: u128, - funds: u128, - direction: FlowType, - now: Timestamp, -) -> Result { - // Only the IBCMODULE can execute transfers - // TODO: Should we move this to a helper method? - // This may not be needed once we move this function to be under sudo. - // Though it might still be worth checking that only the transfer module is calling it - let ibc_module = IBCMODULE.load(deps.storage)?; - if sender != ibc_module { - return Err(ContractError::Unauthorized {}); } - - let trackers = RATE_LIMIT_TRACKERS.may_load(deps.storage, path.into())?; - - let configured = match trackers { - None => false, - Some(ref x) if x.is_empty() => false, - _ => true, - }; - - if !configured { - // No Quota configured for the current path. Allowing all messages. - // TODO: Should there be an attribute for it being allowed vs denied? - return Ok(Response::new() - .add_attribute("method", "try_transfer") - .add_attribute("channel_id", path.channel.to_string()) - .add_attribute("denom", path.denom.to_string()) - .add_attribute("quota", "none")); - } - - let mut rate_limits = trackers.unwrap(); - - // If any of the RateLimits fails, allow_transfer() will return - // ContractError::RateLimitExceded, which we'll propagate out - let results: Vec = rate_limits - .iter_mut() - .map(|limit| limit.allow_transfer(path, &direction, funds, channel_value, now)) - .collect::>()?; - - RATE_LIMIT_TRACKERS.save(deps.storage, path.into(), &results)?; - - let response = Response::new() - .add_attribute("method", "try_transfer") - .add_attribute("channel_id", path.channel.to_string()) - .add_attribute("denom", path.denom.to_string()); - - // Adds the attributes for each path to the response. In prod, the - // addtribute add_rate_limit_attributes is a noop - results.iter().fold(Ok(response), |acc, result| { - Ok(add_rate_limit_attributes(acc?, result)) - }) } -// #[cfg(any(feature = "verbose_responses", test))] -pub fn add_rate_limit_attributes(response: Response, result: &RateLimit) -> Response { - let (used_in, used_out) = result.flow.balance(); - let (max_in, max_out) = result.quota.capacity(); - // These attributes are only added during testing. That way we avoid - // calculating these again on prod. - response - .add_attribute( - format!("{}_used_in", result.quota.name), - used_in.to_string(), - ) - .add_attribute( - format!("{}_used_out", result.quota.name), - used_out.to_string(), - ) - .add_attribute(format!("{}_max_in", result.quota.name), max_in.to_string()) - .add_attribute( - format!("{}_max_out", result.quota.name), - max_out.to_string(), - ) - .add_attribute( - format!("{}_period_end", result.quota.name), - result.flow.period_end.to_string(), - ) -} - -// Leaving the attributes in until we can conditionally compile the contract -// for the go tests in CI: https://github.com/mandrean/cw-optimizoor/issues/19 -// -// #[cfg(not(any(feature = "verbose_responses", test)))] -// pub fn add_rate_limit_attributes(response: Response, _result: &RateLimit) -> Response { -// response -// } - #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { - QueryMsg::GetQuotas { channel_id, denom } => get_quotas(deps, channel_id, denom), + QueryMsg::GetQuotas { channel_id, denom } => query::get_quotas(deps, channel_id, denom), } } -fn get_quotas( - deps: Deps, - channel_id: impl Into, - denom: impl Into, -) -> StdResult { - let path = Path::new(channel_id, denom); - to_binary(&RATE_LIMIT_TRACKERS.load(deps.storage, path.into())?) -} - #[cfg_attr(not(feature = "library"), entry_point)] pub fn migrate(_deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { unimplemented!() } - -#[cfg(test)] -mod tests { - use super::*; - use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info}; - use cosmwasm_std::{from_binary, Addr, Attribute}; - - use crate::helpers::tests::verify_query_response; - use crate::msg::{PathMsg, QuotaMsg}; - use crate::state::{RateLimit, RESET_TIME_WEEKLY}; - - const IBC_ADDR: &str = "IBC_MODULE"; - const GOV_ADDR: &str = "GOV_MODULE"; - - #[test] // Tests we ccan instantiate the contract and that the owners are set correctly - fn proper_instantiation() { - let mut deps = mock_dependencies(); - - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths: vec![], - }; - let info = mock_info(IBC_ADDR, &vec![]); - - // we can just call .unwrap() to assert this was a success - let res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap(); - assert_eq!(0, res.messages.len()); - - // The ibc and gov modules are properly stored - assert_eq!(IBCMODULE.load(deps.as_ref().storage).unwrap(), IBC_ADDR); - assert_eq!(GOVMODULE.load(deps.as_ref().storage).unwrap(), GOV_ADDR); - } - - #[test] // Tests only the IBC_MODULE address can execute send and recv packet - fn permissions() { - let mut deps = mock_dependencies(); - - let quota = QuotaMsg::new("Weekly", RESET_TIME_WEEKLY, 10, 10); - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths: vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![quota], - }], - }; - let info = mock_info(IBC_ADDR, &vec![]); - instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - - // This succeeds - execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - - let info = mock_info("SomeoneElse", &vec![]); - - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let err = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap_err(); - assert!(matches!(err, ContractError::Unauthorized { .. })); - } - - #[test] // Tests that when a packet is transferred, the peropper allowance is consummed - fn consume_allowance() { - let mut deps = mock_dependencies(); - - let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths: vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![quota], - }], - }; - let info = mock_info(GOV_ADDR, &vec![]); - let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let info = mock_info(IBC_ADDR, &vec![]); - let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "300"); - - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let err = execute(deps.as_mut(), mock_env(), info, msg).unwrap_err(); - assert!(matches!(err, ContractError::RateLimitExceded { .. })); - } - - #[test] // Tests that the balance of send and receive is maintained (i.e: recives are sustracted from the send allowance and sends from the receives) - fn symetric_flows_dont_consume_allowance() { - let mut deps = mock_dependencies(); - - let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths: vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![quota], - }], - }; - let info = mock_info(GOV_ADDR, &vec![]); - let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - - let info = mock_info(IBC_ADDR, &vec![]); - let send_msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let recv_msg = ExecuteMsg::RecvPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - - let res = execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used_in"); - assert_eq!(value, "0"); - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "300"); - - let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used_in"); - assert_eq!(value, "0"); - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "0"); - - // We can still use the path. Even if we have sent more than the - // allowance through the path (900 > 3000*.1), the current "balance" - // of inflow vs outflow is still lower than the path's capacity/quota - let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used_in"); - assert_eq!(value, "300"); - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "0"); - - let err = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg.clone()).unwrap_err(); - - assert!(matches!(err, ContractError::RateLimitExceded { .. })); - } - - #[test] // Tests that we can have different quotas for send and receive. In this test we use 4% send and 1% receive - fn asymetric_quotas() { - let mut deps = mock_dependencies(); - - let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 4, 1); - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths: vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![quota], - }], - }; - let info = mock_info(GOV_ADDR, &vec![]); - let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - - // Sending 2% - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 60, - }; - let info = mock_info(IBC_ADDR, &vec![]); - let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "60"); - - // Sending 2% more. Allowed, as sending has a 4% allowance - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 60, - }; - - let info = mock_info(IBC_ADDR, &vec![]); - let res = execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); - println!("{res:?}"); - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "120"); - - // Receiving 1% should still work. 4% *sent* through the path, but we can still receive. - let recv_msg = ExecuteMsg::RecvPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 30, - }; - let res = execute(deps.as_mut(), mock_env(), info.clone(), recv_msg).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used_in"); - assert_eq!(value, "0"); - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "90"); - - // Sending 2%. Should fail. In balance, we've sent 4% and received 1%, so only 1% left to send. - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 60, - }; - let err = execute(deps.as_mut(), mock_env(), info.clone(), msg.clone()).unwrap_err(); - assert!(matches!(err, ContractError::RateLimitExceded { .. })); - - // Sending 1%: Allowed; because sending has a 4% allowance. We've sent 4% already, but received 1%, so there's send cappacity again - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 30, - }; - let res = execute(deps.as_mut(), mock_env(), info.clone(), msg.clone()).unwrap(); - let Attribute { key, value } = &res.attributes[3]; - assert_eq!(key, "weekly_used_in"); - assert_eq!(value, "0"); - let Attribute { key, value } = &res.attributes[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "120"); - } - - #[test] // Tests we can get the current state of the trackers - fn query_state() { - let mut deps = mock_dependencies(); - - let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths: vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![quota], - }], - }; - let info = mock_info(GOV_ADDR, &vec![]); - let env = mock_env(); - let _res = instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); - - let query_msg = QueryMsg::GetQuotas { - channel_id: format!("channel"), - denom: format!("denom"), - }; - - let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); - let value: Vec = from_binary(&res).unwrap(); - assert_eq!(value[0].quota.name, "weekly"); - assert_eq!(value[0].quota.max_percentage_send, 10); - assert_eq!(value[0].quota.max_percentage_recv, 10); - assert_eq!(value[0].quota.duration, RESET_TIME_WEEKLY); - assert_eq!(value[0].flow.inflow, 0); - assert_eq!(value[0].flow.outflow, 0); - assert_eq!( - value[0].flow.period_end, - env.block.time.plus_seconds(RESET_TIME_WEEKLY) - ); - - let info = mock_info(IBC_ADDR, &vec![]); - let send_msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - execute(deps.as_mut(), mock_env(), info.clone(), send_msg.clone()).unwrap(); - - let recv_msg = ExecuteMsg::RecvPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 30, - }; - execute(deps.as_mut(), mock_env(), info, recv_msg.clone()).unwrap(); - - // Query - let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); - let value: Vec = from_binary(&res).unwrap(); - verify_query_response( - &value[0], - "weekly", - (10, 10), - RESET_TIME_WEEKLY, - 30, - 300, - env.block.time.plus_seconds(RESET_TIME_WEEKLY), - ); - } - - #[test] // Tests quota percentages are between [0,100] - fn bad_quotas() { - let mut deps = mock_dependencies(); - - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths: vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![QuotaMsg { - name: "bad_quota".to_string(), - duration: 200, - send_recv: (5000, 101), - }], - }], - }; - let info = mock_info(IBC_ADDR, &vec![]); - - let env = mock_env(); - instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); - - // If a quota is higher than 100%, we set it to 100% - let query_msg = QueryMsg::GetQuotas { - channel_id: format!("channel"), - denom: format!("denom"), - }; - let res = query(deps.as_ref(), env.clone(), query_msg).unwrap(); - let value: Vec = from_binary(&res).unwrap(); - verify_query_response( - &value[0], - "bad_quota", - (100, 100), - 200, - 0, - 0, - env.block.time.plus_seconds(200), - ); - } -} 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 new file mode 100644 index 00000000000..f35d41fc108 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract_tests.rs @@ -0,0 +1,324 @@ +#![cfg(test)] + +use crate::{contract::*, ContractError}; +use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info}; +use cosmwasm_std::{from_binary, Addr, Attribute}; + +use crate::helpers::tests::verify_query_response; +use crate::msg::{InstantiateMsg, PathMsg, QueryMsg, QuotaMsg, SudoMsg}; +use crate::state::tests::RESET_TIME_WEEKLY; +use crate::state::{RateLimit, GOVMODULE, IBCMODULE}; + +const IBC_ADDR: &str = "IBC_MODULE"; +const GOV_ADDR: &str = "GOV_MODULE"; + +#[test] // Tests we ccan instantiate the contract and that the owners are set correctly +fn proper_instantiation() { + let mut deps = mock_dependencies(); + + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + paths: vec![], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + // we can just call .unwrap() to assert this was a success + let res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap(); + assert_eq!(0, res.messages.len()); + + // The ibc and gov modules are properly stored + assert_eq!(IBCMODULE.load(deps.as_ref().storage).unwrap(), IBC_ADDR); + assert_eq!(GOVMODULE.load(deps.as_ref().storage).unwrap(), GOV_ADDR); +} + +#[test] // Tests that when a packet is transferred, the peropper allowance is consummed +fn consume_allowance() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap(); + + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let res = sudo(deps.as_mut(), mock_env(), msg).unwrap(); + + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "300"); + + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let err = sudo(deps.as_mut(), mock_env(), msg).unwrap_err(); + assert!(matches!(err, ContractError::RateLimitExceded { .. })); +} + +#[test] // Tests that the balance of send and receive is maintained (i.e: recives are sustracted from the send allowance and sends from the receives) +fn symetric_flows_dont_consume_allowance() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + + let send_msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let recv_msg = SudoMsg::RecvPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + + let res = sudo(deps.as_mut(), mock_env(), send_msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "300"); + + let res = sudo(deps.as_mut(), mock_env(), recv_msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "0"); + + // We can still use the path. Even if we have sent more than the + // allowance through the path (900 > 3000*.1), the current "balance" + // of inflow vs outflow is still lower than the path's capacity/quota + let res = sudo(deps.as_mut(), mock_env(), recv_msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "0"); + + let err = sudo(deps.as_mut(), mock_env(), recv_msg.clone()).unwrap_err(); + + assert!(matches!(err, ContractError::RateLimitExceded { .. })); +} + +#[test] // Tests that we can have different quotas for send and receive. In this test we use 4% send and 1% receive +fn asymetric_quotas() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 4, 1); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let _res = instantiate(deps.as_mut(), mock_env(), info.clone(), msg).unwrap(); + + // Sending 2% + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 60, + }; + let res = sudo(deps.as_mut(), mock_env(), msg).unwrap(); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "60"); + + // Sending 2% more. Allowed, as sending has a 4% allowance + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 60, + }; + + let res = sudo(deps.as_mut(), mock_env(), msg).unwrap(); + println!("{res:?}"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "120"); + + // Receiving 1% should still work. 4% *sent* through the path, but we can still receive. + let recv_msg = SudoMsg::RecvPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 30, + }; + let res = sudo(deps.as_mut(), mock_env(), recv_msg).unwrap(); + let Attribute { key, value } = &res.attributes[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "90"); + + // Sending 2%. Should fail. In balance, we've sent 4% and received 1%, so only 1% left to send. + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 60, + }; + let err = sudo(deps.as_mut(), mock_env(), msg.clone()).unwrap_err(); + assert!(matches!(err, ContractError::RateLimitExceded { .. })); + + // Sending 1%: Allowed; because sending has a 4% allowance. We've sent 4% already, but received 1%, so there's send cappacity again + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 30, + }; + let res = sudo(deps.as_mut(), mock_env(), msg.clone()).unwrap(); + let Attribute { key, value } = &res.attributes[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.attributes[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "120"); +} + +#[test] // Tests we can get the current state of the trackers +fn query_state() { + let mut deps = mock_dependencies(); + + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![quota], + }], + }; + let info = mock_info(GOV_ADDR, &vec![]); + let env = mock_env(); + let _res = instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); + + let query_msg = QueryMsg::GetQuotas { + channel_id: format!("channel"), + denom: format!("denom"), + }; + + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + assert_eq!(value[0].quota.name, "weekly"); + assert_eq!(value[0].quota.max_percentage_send, 10); + assert_eq!(value[0].quota.max_percentage_recv, 10); + assert_eq!(value[0].quota.duration, RESET_TIME_WEEKLY); + assert_eq!(value[0].flow.inflow, 0); + assert_eq!(value[0].flow.outflow, 0); + assert_eq!( + value[0].flow.period_end, + env.block.time.plus_seconds(RESET_TIME_WEEKLY) + ); + + let send_msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + sudo(deps.as_mut(), mock_env(), send_msg.clone()).unwrap(); + + let recv_msg = SudoMsg::RecvPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 30, + }; + sudo(deps.as_mut(), mock_env(), recv_msg.clone()).unwrap(); + + // Query + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + verify_query_response( + &value[0], + "weekly", + (10, 10), + RESET_TIME_WEEKLY, + 30, + 300, + env.block.time.plus_seconds(RESET_TIME_WEEKLY), + ); +} + +#[test] // Tests quota percentages are between [0,100] +fn bad_quotas() { + let mut deps = mock_dependencies(); + + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + paths: vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![QuotaMsg { + name: "bad_quota".to_string(), + duration: 200, + send_recv: (5000, 101), + }], + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + let env = mock_env(); + instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); + + // If a quota is higher than 100%, we set it to 100% + let query_msg = QueryMsg::GetQuotas { + channel_id: format!("channel"), + denom: format!("denom"), + }; + let res = query(deps.as_ref(), env.clone(), query_msg).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + verify_query_response( + &value[0], + "bad_quota", + (100, 100), + 200, + 0, + 0, + env.block.time.plus_seconds(200), + ); +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/execute.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/execute.rs new file mode 100644 index 00000000000..4bac4c0d37f --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/execute.rs @@ -0,0 +1,249 @@ +use crate::msg::{PathMsg, QuotaMsg}; +use crate::state::{Flow, Path, RateLimit, GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS}; +use crate::ContractError; +use cosmwasm_std::{Addr, DepsMut, Response, Timestamp}; + +pub fn add_new_paths( + deps: DepsMut, + path_msgs: Vec, + now: Timestamp, +) -> Result<(), ContractError> { + for path_msg in path_msgs { + let path = Path::new(path_msg.channel_id, path_msg.denom); + + RATE_LIMIT_TRACKERS.save( + deps.storage, + path.into(), + &path_msg + .quotas + .iter() + .map(|q| RateLimit { + quota: q.into(), + flow: Flow::new(0_u128, 0_u128, now, q.duration), + }) + .collect(), + )? + } + Ok(()) +} + +pub fn try_add_path( + deps: DepsMut, + sender: Addr, + channel_id: String, + denom: String, + quotas: Vec, + now: Timestamp, +) -> Result { + // codenit: should we make a function for checking this authorization? + let ibc_module = IBCMODULE.load(deps.storage)?; + let gov_module = GOVMODULE.load(deps.storage)?; + if sender != ibc_module && sender != gov_module { + return Err(ContractError::Unauthorized {}); + } + add_new_paths(deps, vec![PathMsg::new(&channel_id, &denom, quotas)], now)?; + + Ok(Response::new() + .add_attribute("method", "try_add_channel") + .add_attribute("channel_id", channel_id) + .add_attribute("denom", denom)) +} + +pub fn try_remove_path( + deps: DepsMut, + sender: Addr, + channel_id: String, + denom: String, +) -> Result { + let ibc_module = IBCMODULE.load(deps.storage)?; + let gov_module = GOVMODULE.load(deps.storage)?; + if sender != ibc_module && sender != gov_module { + return Err(ContractError::Unauthorized {}); + } + + let path = Path::new(&channel_id, &denom); + RATE_LIMIT_TRACKERS.remove(deps.storage, path.into()); + Ok(Response::new() + .add_attribute("method", "try_remove_channel") + .add_attribute("denom", denom) + .add_attribute("channel_id", channel_id)) +} + +// Reset specified quote_id for the given channel_id +pub fn try_reset_path_quota( + deps: DepsMut, + sender: Addr, + channel_id: String, + denom: String, + quota_id: String, + now: Timestamp, +) -> Result { + let gov_module = GOVMODULE.load(deps.storage)?; + if sender != gov_module { + return Err(ContractError::Unauthorized {}); + } + + let path = Path::new(&channel_id, &denom); + RATE_LIMIT_TRACKERS.update(deps.storage, path.into(), |maybe_rate_limit| { + match maybe_rate_limit { + None => Err(ContractError::QuotaNotFound { + quota_id, + channel_id: channel_id.clone(), + denom: denom.clone(), + }), + Some(mut limits) => { + // Q: What happens here if quote_id not found? seems like we return ok? + limits.iter_mut().for_each(|limit| { + if limit.quota.name == quota_id.as_ref() { + limit.flow.expire(now, limit.quota.duration) + } + }); + Ok(limits) + } + } + })?; + + Ok(Response::new() + .add_attribute("method", "try_reset_channel") + .add_attribute("channel_id", channel_id)) +} + +#[cfg(test)] +mod tests { + use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info}; + use cosmwasm_std::{from_binary, Addr, StdError}; + + use crate::contract::{execute, query}; + use crate::helpers::tests::verify_query_response; + use crate::msg::{ExecuteMsg, QueryMsg, QuotaMsg}; + use crate::state::{RateLimit, GOVMODULE, IBCMODULE}; + + const IBC_ADDR: &str = "IBC_MODULE"; + const GOV_ADDR: &str = "GOV_MODULE"; + + #[test] // Tests AddPath and RemovePath messages + fn management_add_and_remove_path() { + let mut deps = mock_dependencies(); + IBCMODULE + .save(deps.as_mut().storage, &Addr::unchecked(IBC_ADDR)) + .unwrap(); + GOVMODULE + .save(deps.as_mut().storage, &Addr::unchecked(GOV_ADDR)) + .unwrap(); + + let msg = ExecuteMsg::AddPath { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![QuotaMsg { + name: "daily".to_string(), + duration: 1600, + send_recv: (3, 5), + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + let env = mock_env(); + let res = execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + assert_eq!(0, res.messages.len()); + + let query_msg = QueryMsg::GetQuotas { + channel_id: format!("channel"), + denom: format!("denom"), + }; + + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + + let value: Vec = from_binary(&res).unwrap(); + verify_query_response( + &value[0], + "daily", + (3, 5), + 1600, + 0, + 0, + env.block.time.plus_seconds(1600), + ); + + assert_eq!(value.len(), 1); + + // Add another path + let msg = ExecuteMsg::AddPath { + channel_id: format!("channel2"), + denom: format!("denom"), + quotas: vec![QuotaMsg { + name: "daily".to_string(), + duration: 1600, + send_recv: (3, 5), + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + let env = mock_env(); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + // remove the first one + let msg = ExecuteMsg::RemovePath { + channel_id: format!("channel"), + denom: format!("denom"), + }; + + let info = mock_info(IBC_ADDR, &vec![]); + let env = mock_env(); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + // The channel is not there anymore + let err = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap_err(); + assert!(matches!(err, StdError::NotFound { .. })); + + // The second channel is still there + let query_msg = QueryMsg::GetQuotas { + channel_id: format!("channel2"), + denom: format!("denom"), + }; + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + assert_eq!(value.len(), 1); + verify_query_response( + &value[0], + "daily", + (3, 5), + 1600, + 0, + 0, + env.block.time.plus_seconds(1600), + ); + + // Paths are overriden if they share a name and denom + let msg = ExecuteMsg::AddPath { + channel_id: format!("channel2"), + denom: format!("denom"), + quotas: vec![QuotaMsg { + name: "different".to_string(), + duration: 5000, + send_recv: (50, 30), + }], + }; + let info = mock_info(IBC_ADDR, &vec![]); + + let env = mock_env(); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + let query_msg = QueryMsg::GetQuotas { + channel_id: format!("channel2"), + denom: format!("denom"), + }; + let res = query(deps.as_ref(), mock_env(), query_msg.clone()).unwrap(); + let value: Vec = from_binary(&res).unwrap(); + assert_eq!(value.len(), 1); + + verify_query_response( + &value[0], + "different", + (50, 30), + 5000, + 0, + 0, + env.block.time.plus_seconds(5000), + ); + } +} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs index 092b24508cd..6cfd60a65a8 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/helpers.rs @@ -1,9 +1,11 @@ +#![cfg(test)] use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use cosmwasm_std::{to_binary, Addr, CosmosMsg, StdResult, WasmMsg}; use crate::msg::ExecuteMsg; +use crate::msg::SudoMsg; /// CwTemplateContract is a wrapper around Addr that provides a lot of helpers /// for working with this. @@ -24,9 +26,16 @@ impl RateLimitingContract { } .into()) } + + pub fn sudo>(&self, msg: T) -> cw_multi_test::SudoMsg { + let msg = to_binary(&msg.into()).unwrap(); + cw_multi_test::SudoMsg::Wasm(cw_multi_test::WasmSudo { + contract_addr: self.addr().into(), + msg, + }) + } } -#[cfg(test)] pub mod tests { use cosmwasm_std::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 3c4ccdae697..8807028fcb9 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 @@ -1,187 +1,202 @@ -#[cfg(test)] -mod tests { - use crate::helpers::RateLimitingContract; - use cosmwasm_std::{Addr, Coin, Empty, Uint128}; - use cw_multi_test::{App, AppBuilder, Contract, ContractWrapper, Executor}; - - use crate::{ - msg::{ExecuteMsg, InstantiateMsg, PathMsg, QuotaMsg}, - state::{RESET_TIME_DAILY, RESET_TIME_MONTHLY, RESET_TIME_WEEKLY}, - }; - - pub fn contract_template() -> Box> { - let contract = ContractWrapper::new( - crate::contract::execute, - crate::contract::instantiate, - crate::contract::query, - ); - Box::new(contract) - } - - const USER: &str = "USER"; - const IBC_ADDR: &str = "IBC_MODULE"; - const GOV_ADDR: &str = "GOV_MODULE"; - const NATIVE_DENOM: &str = "nosmo"; - - fn mock_app() -> App { - AppBuilder::new().build(|router, _, storage| { - router - .bank - .init_balance( - storage, - &Addr::unchecked(USER), - vec![Coin { - denom: NATIVE_DENOM.to_string(), - amount: Uint128::new(1_000), - }], - ) - .unwrap(); - }) - } - - // Instantiate the contract - fn proper_instantiate(paths: Vec) -> (App, RateLimitingContract) { - let mut app = mock_app(); - let cw_template_id = app.store_code(contract_template()); - - let msg = InstantiateMsg { - gov_module: Addr::unchecked(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), - paths, - }; +#![cfg(test)] +use crate::{helpers::RateLimitingContract, msg::ExecuteMsg}; +use cosmwasm_std::{Addr, Coin, Empty, Uint128}; +use cw_multi_test::{App, AppBuilder, Contract, ContractWrapper, Executor}; + +use crate::{ + msg::{InstantiateMsg, PathMsg, QuotaMsg, SudoMsg}, + state::tests::{RESET_TIME_DAILY, RESET_TIME_MONTHLY, RESET_TIME_WEEKLY}, +}; + +pub fn contract_template() -> Box> { + let contract = ContractWrapper::new( + crate::contract::execute, + crate::contract::instantiate, + crate::contract::query, + ) + .with_sudo(crate::contract::sudo); + Box::new(contract) +} - let cw_template_contract_addr = app - .instantiate_contract( - cw_template_id, - Addr::unchecked(GOV_ADDR), - &msg, - &[], - "test", - None, +const USER: &str = "USER"; +const IBC_ADDR: &str = "IBC_MODULE"; +const GOV_ADDR: &str = "GOV_MODULE"; +const NATIVE_DENOM: &str = "nosmo"; + +fn mock_app() -> App { + AppBuilder::new().build(|router, _, storage| { + router + .bank + .init_balance( + storage, + &Addr::unchecked(USER), + vec![Coin { + denom: NATIVE_DENOM.to_string(), + amount: Uint128::new(1_000), + }], ) .unwrap(); + }) +} - let cw_template_contract = RateLimitingContract(cw_template_contract_addr); - - (app, cw_template_contract) - } +// Instantiate the contract +fn proper_instantiate(paths: Vec) -> (App, RateLimitingContract) { + let mut app = mock_app(); + let cw_template_id = app.store_code(contract_template()); - use cosmwasm_std::Attribute; + let msg = InstantiateMsg { + gov_module: Addr::unchecked(GOV_ADDR), + ibc_module: Addr::unchecked(IBC_ADDR), + paths, + }; - #[test] // Checks that the RateLimit flows are expired properly when time passes - fn expiration() { - let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); + let cw_rate_limit_contract_addr = app + .instantiate_contract( + cw_template_id, + Addr::unchecked(GOV_ADDR), + &msg, + &[], + "test", + None, + ) + .unwrap(); - let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas: vec![quota], - }]); - - // Using all the allowance - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - let Attribute { key, value } = &res.custom_attrs(1)[3]; - assert_eq!(key, "weekly_used_in"); - assert_eq!(value, "0"); - let Attribute { key, value } = &res.custom_attrs(1)[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[5]; - assert_eq!(key, "weekly_max_in"); - assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[6]; - assert_eq!(key, "weekly_max_out"); - assert_eq!(value, "300"); - - // Another packet is rate limited - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); + let cw_rate_limit_contract = RateLimitingContract(cw_rate_limit_contract_addr); - // TODO: how do we check the error type here? + (app, cw_rate_limit_contract) +} - // ... Time passes - app.update_block(|b| { - b.height += 1000; - b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) - }); +use cosmwasm_std::Attribute; - // Sending the packet should work now - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 3_000, - funds: 300, - }; +#[test] // Checks that the RateLimit flows are expired properly when time passes +fn expiration() { + let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10); - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - let Attribute { key, value } = &res.custom_attrs(1)[3]; - assert_eq!(key, "weekly_used_in"); - assert_eq!(value, "0"); - let Attribute { key, value } = &res.custom_attrs(1)[4]; - assert_eq!(key, "weekly_used_out"); - assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[5]; - assert_eq!(key, "weekly_max_in"); - assert_eq!(value, "300"); - let Attribute { key, value } = &res.custom_attrs(1)[6]; - assert_eq!(key, "weekly_max_out"); - assert_eq!(value, "300"); - } + let (mut app, cw_rate_limit_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![quota], + }]); - #[test] // Tests we can have different maximums for different quotaas (daily, weekly, etc) and that they all are active at the same time - fn multiple_quotas() { - let quotas = vec![ - QuotaMsg::new("daily", RESET_TIME_DAILY, 1, 1), - QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), - QuotaMsg::new("monthly", RESET_TIME_MONTHLY, 5, 5), - ]; + // Using all the allowance + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + let res = app.sudo(cosmos_msg).unwrap(); + + let Attribute { key, value } = &res.custom_attrs(1)[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.custom_attrs(1)[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[5]; + assert_eq!(key, "weekly_max_in"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[6]; + assert_eq!(key, "weekly_max_out"); + assert_eq!(value, "300"); + + // Another packet is rate limited + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + let _err = app.sudo(cosmos_msg).unwrap_err(); + + // TODO: how do we check the error type here? + + // ... Time passes + app.update_block(|b| { + b.height += 1000; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // Sending the packet should work now + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; - let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas, - }]); + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + let res = app.sudo(cosmos_msg).unwrap(); + + let Attribute { key, value } = &res.custom_attrs(1)[3]; + assert_eq!(key, "weekly_used_in"); + assert_eq!(value, "0"); + let Attribute { key, value } = &res.custom_attrs(1)[4]; + assert_eq!(key, "weekly_used_out"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[5]; + assert_eq!(key, "weekly_max_in"); + assert_eq!(value, "300"); + let Attribute { key, value } = &res.custom_attrs(1)[6]; + assert_eq!(key, "weekly_max_out"); + assert_eq!(value, "300"); +} - // Sending 1% to use the daily allowance - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); +#[test] // Tests we can have different maximums for different quotaas (daily, weekly, etc) and that they all are active at the same time +fn multiple_quotas() { + let quotas = vec![ + QuotaMsg::new("daily", RESET_TIME_DAILY, 1, 1), + QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), + QuotaMsg::new("monthly", RESET_TIME_MONTHLY, 5, 5), + ]; + + let (mut app, cw_rate_limit_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas, + }]); + + // Sending 1% to use the daily allowance + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap(); + + // Another packet is rate limited + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // Sending the packet should work now + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; - // Another packet is rate limited - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap(); + // Do that for 4 more days + for _ in 1..4 { // ... One day passes app.update_block(|b| { b.height += 10; @@ -189,201 +204,202 @@ mod tests { }); // Sending the packet should work now - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - // Do that for 4 more days - for _ in 1..4 { - // ... One day passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) - }); - - // Sending the packet should work now - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - } - - // ... One day passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) - }); - - // We now have exceeded the weekly limit! Even if the daily limit allows us, the weekly doesn't - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // ... One week passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) - }); - - // We can still can't send because the weekly and monthly limits are the same - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // Waiting a week again, doesn't help!! - // ... One week passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) - }); - - // We can still can't send because the monthly limit hasn't passed - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // Only after two more weeks we can send again - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds((RESET_TIME_WEEKLY * 2) + 1) // Two weeks - }); - - let msg = ExecuteMsg::SendPacket { + let msg = SudoMsg::SendPacket { channel_id: format!("channel"), denom: format!("denom"), channel_value: 100, funds: 1, }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap(); } - #[test] // Tests that the channel value is based on the value at the beginning of the period - fn channel_value_cached() { - let quotas = vec![ - QuotaMsg::new("daily", RESET_TIME_DAILY, 2, 2), - QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), - ]; - - let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { - channel_id: format!("channel"), - denom: format!("denom"), - quotas, - }]); - - // Sending 1% (half of the daily allowance) - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 1, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - - // Sending 3% is now rate limited - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100, - funds: 3, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); - - // Even if the channel value increases, the percentage is calculated based on the value at period start - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 100000, - funds: 3, - }; - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - let _err = app - .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // We now have exceeded the weekly limit! Even if the daily limit allows us, the weekly doesn't + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); + + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // We can still can't send because the weekly and monthly limits are the same + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); + + // Waiting a week again, doesn't help!! + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // We can still can't send because the monthly limit hasn't passed + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); + + // Only after two more weeks we can send again + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds((RESET_TIME_WEEKLY * 2) + 1) // Two weeks + }); + + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap(); +} - // ... One day passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) - }); +#[test] // Tests that the channel value is based on the value at the beginning of the period +fn channel_value_cached() { + let quotas = vec![ + QuotaMsg::new("daily", RESET_TIME_DAILY, 2, 2), + QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), + ]; + + let (mut app, cw_rate_limit_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas, + }]); + + // Sending 1% (half of the daily allowance) + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap(); + + // Sending 3% is now rate limited + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 3, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); + + // Even if the channel value increases, the percentage is calculated based on the value at period start + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100000, + funds: 3, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // New Channel Value world! + + // Sending 1% of a new value (10_000) passes the daily check, cause it + // has expired, but not the weekly check (The value for last week is + // sitll 100, as only 1 day has passed) + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 10_000, + funds: 100, + }; - // New Channel Value world! + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); + + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // Sending 1% of a new value should work and set the value for the day at 10_000 + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 10_000, + funds: 100, + }; - // Sending 1% of a new value (10_000) passes the daily check, cause it - // has expired, but not the weekly check (The value for last week is - // sitll 100, as only 1 day has passed) - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 10_000, - funds: 100, - }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap(); - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg) - .unwrap_err(); + // If the value magically decreasses. We can still send up to 100 more (1% of 10k) + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 1, + funds: 75, + }; - // ... One week passes - app.update_block(|b| { - b.height += 10; - b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) - }); + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap(); +} - // Sending 1% of a new value should work and set the value for the day at 10_000 - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 10_000, - funds: 100, - }; +#[test] // Checks that RateLimits added after instantiation are respected +fn add_paths_later() { + let (mut app, cw_rate_limit_contract) = proper_instantiate(vec![]); - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + // All sends are allowed + let msg = SudoMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 3_000, + funds: 300, + }; + let cosmos_msg = cw_rate_limit_contract.sudo(msg.clone()); + let res = app.sudo(cosmos_msg).unwrap(); + let Attribute { key, value } = &res.custom_attrs(1)[3]; + assert_eq!(key, "quota"); + assert_eq!(value, "none"); + + // Add a weekly limit of 1% + let management_msg = ExecuteMsg::AddPath { + channel_id: format!("channel"), + denom: format!("denom"), + quotas: vec![QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 1, 1)], + }; - // If the value magically decreasses. We can still send up to 100 more (1% of 10k) - let msg = ExecuteMsg::SendPacket { - channel_id: format!("channel"), - denom: format!("denom"), - channel_value: 1, - funds: 75, - }; + let cosmos_msg = cw_rate_limit_contract.call(management_msg).unwrap(); + app.execute(Addr::unchecked(GOV_ADDR), cosmos_msg).unwrap(); - let cosmos_msg = cw_template_contract.call(msg).unwrap(); - app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); - } + // Executing the same message again should fail, as it is now rate limited + let cosmos_msg = cw_rate_limit_contract.sudo(msg); + app.sudo(cosmos_msg).unwrap_err(); } diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs index 573d76512d7..0b7ddb6b66f 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/lib.rs @@ -1,9 +1,17 @@ +// Contract pub mod contract; mod error; -pub mod helpers; -pub mod integration_tests; -pub mod management; pub mod msg; -pub mod state; +mod state; + +// Functions +mod execute; +mod query; +mod sudo; + +// Tests +mod contract_tests; +mod helpers; +mod integration_tests; pub use crate::error::ContractError; diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index be0ba35e2f9..bbbf6803389 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -56,18 +56,6 @@ pub struct InstantiateMsg { #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum ExecuteMsg { - SendPacket { - channel_id: String, - denom: String, - channel_value: u128, - funds: u128, - }, - RecvPacket { - channel_id: String, - denom: String, - channel_value: u128, - funds: u128, - }, AddPath { channel_id: String, denom: String, @@ -90,6 +78,23 @@ pub enum QueryMsg { GetQuotas { channel_id: String, denom: String }, } +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum SudoMsg { + SendPacket { + channel_id: String, + denom: String, + channel_value: u128, + funds: u128, + }, + RecvPacket { + channel_id: String, + denom: String, + channel_value: u128, + funds: u128, + }, +} + #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum MigrateMsg {} diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/query.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/query.rs new file mode 100644 index 00000000000..6431a837d47 --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/query.rs @@ -0,0 +1,12 @@ +use cosmwasm_std::{to_binary, Binary, Deps, StdResult}; + +use crate::state::{Path, RATE_LIMIT_TRACKERS}; + +pub fn get_quotas( + deps: Deps, + channel_id: impl Into, + denom: impl Into, +) -> StdResult { + let path = Path::new(channel_id, denom); + to_binary(&RATE_LIMIT_TRACKERS.load(deps.storage, path.into())?) +} 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 b534ccfd1b1..73dba1d7f72 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -7,10 +7,6 @@ use cw_storage_plus::{Item, Map}; use crate::{msg::QuotaMsg, ContractError}; -pub const RESET_TIME_DAILY: u64 = 60 * 60 * 24; -pub const RESET_TIME_WEEKLY: u64 = 60 * 60 * 24 * 7; -pub const RESET_TIME_MONTHLY: u64 = 60 * 60 * 24 * 30; - /// This represents the key for our rate limiting tracker. A tuple of a denom and /// a channel. When interactic with storage, it's preffered to use this struct /// and call path.into() on it to convert it to the composite key of the @@ -271,9 +267,13 @@ pub const IBCMODULE: Item = Item::new("ibc_module"); pub const RATE_LIMIT_TRACKERS: Map<(String, String), Vec> = Map::new("flow"); #[cfg(test)] -mod tests { +pub mod tests { use super::*; + pub const RESET_TIME_DAILY: u64 = 60 * 60 * 24; + pub const RESET_TIME_WEEKLY: u64 = 60 * 60 * 24 * 7; + pub const RESET_TIME_MONTHLY: u64 = 60 * 60 * 24 * 30; + #[test] fn flow() { let epoch = Timestamp::from_seconds(0); diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/sudo.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/sudo.rs new file mode 100644 index 00000000000..8df8398965c --- /dev/null +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/sudo.rs @@ -0,0 +1,95 @@ +use cosmwasm_std::{DepsMut, Response, Timestamp}; + +use crate::{ + state::{FlowType, Path, RateLimit, RATE_LIMIT_TRACKERS}, + ContractError, +}; + +/// This function checks the rate limit and, if successful, stores the updated data about the value +/// that has been transfered through the channel for a specific denom. +/// If the period for a RateLimit has ended, the Flow information is reset. +/// +/// The channel_value is the current value of the denom for the the channel as +/// calculated by the caller. This should be the total supply of a denom +pub fn try_transfer( + deps: DepsMut, + path: &Path, + channel_value: u128, + funds: u128, + direction: FlowType, + now: Timestamp, +) -> Result { + // Sudo call. Only go modules should be allowed to access this + let trackers = RATE_LIMIT_TRACKERS.may_load(deps.storage, path.into())?; + + let configured = match trackers { + None => false, + Some(ref x) if x.is_empty() => false, + _ => true, + }; + + if !configured { + // No Quota configured for the current path. Allowing all messages. + return Ok(Response::new() + .add_attribute("method", "try_transfer") + .add_attribute("channel_id", path.channel.to_string()) + .add_attribute("denom", path.denom.to_string()) + .add_attribute("quota", "none")); + } + + let mut rate_limits = trackers.unwrap(); + + // If any of the RateLimits fails, allow_transfer() will return + // ContractError::RateLimitExceded, which we'll propagate out + let results: Vec = rate_limits + .iter_mut() + .map(|limit| limit.allow_transfer(path, &direction, funds, channel_value, now)) + .collect::>()?; + + RATE_LIMIT_TRACKERS.save(deps.storage, path.into(), &results)?; + + let response = Response::new() + .add_attribute("method", "try_transfer") + .add_attribute("channel_id", path.channel.to_string()) + .add_attribute("denom", path.denom.to_string()); + + // Adds the attributes for each path to the response. In prod, the + // addtribute add_rate_limit_attributes is a noop + results.iter().fold(Ok(response), |acc, result| { + Ok(add_rate_limit_attributes(acc?, result)) + }) +} + +// #[cfg(any(feature = "verbose_responses", test))] +fn add_rate_limit_attributes(response: Response, result: &RateLimit) -> Response { + let (used_in, used_out) = result.flow.balance(); + let (max_in, max_out) = result.quota.capacity(); + // These attributes are only added during testing. That way we avoid + // calculating these again on prod. + response + .add_attribute( + format!("{}_used_in", result.quota.name), + used_in.to_string(), + ) + .add_attribute( + format!("{}_used_out", result.quota.name), + used_out.to_string(), + ) + .add_attribute(format!("{}_max_in", result.quota.name), max_in.to_string()) + .add_attribute( + format!("{}_max_out", result.quota.name), + max_out.to_string(), + ) + .add_attribute( + format!("{}_period_end", result.quota.name), + result.flow.period_end.to_string(), + ) +} + +// Leaving the attributes in until we can conditionally compile the contract +// for the go tests in CI: https://github.com/mandrean/cw-optimizoor/issues/19 +// +// #[cfg(not(any(feature = "verbose_responses", test)))] +// fn add_rate_limit_attributes(response: Response, _result: &RateLimit) -> Response { +// response +// } From 5cc1943e5fc98e6fed5e6873229b47e4d81fe213 Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Mon, 22 Aug 2022 12:57:45 +0200 Subject: [PATCH 29/29] lint --- x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs index bbbf6803389..b801537c23a 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/msg.rs @@ -78,7 +78,7 @@ pub enum QueryMsg { GetQuotas { channel_id: String, denom: String }, } -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum SudoMsg { SendPacket {